Speed up warmServer by loading pages (and files) asynchronously (gith…

…ub#16752) * Async `new Page` * Update pages.js * Update pages.js * Update pages.js * Update pages.js * Update pages.js * Using mapLimit * Update pages.js * Test updates * Update pages.js * Move exists to page class * Test fixes * Slightly faster localized paths process
milad237 · Dec 9, 2020 · 1d5e216 · 1d5e216
1 parent ee7c1bc
commit 1d5e216
Show file tree

Hide file tree

Showing 10 changed files with 142 additions and 103 deletions.
diff --git a/lib/page.js b/lib/page.js
@@ -1,5 +1,5 @@
 const assert = require('assert')
-const fs = require('fs')
+const fs = require('fs').promises
 const path = require('path')
 const cheerio = require('cheerio')
 const patterns = require('./patterns')
@@ -23,15 +23,30 @@ const slash = require('slash')
 const statsd = require('./statsd')
 
 class Page {
-  constructor (opts) {
+  static async init (opts) {
     assert(opts.relativePath, 'relativePath is required')
     assert(opts.basePath, 'basePath is required')
+
+    const relativePath = slash(opts.relativePath)
+    const fullPath = slash(path.join(opts.basePath, relativePath))
+    const raw = await fs.readFile(fullPath, 'utf8')
+
+    return new Page({ ...opts, relativePath, fullPath, raw })
+  }
+
+  static async exists (path) {
+    try {
+      return await fs.stat(path)
+    } catch (err) {
+      if (err.code === 'ENOENT') return false
+      console.error(err)
+    }
+  }
+
+  constructor (opts) {
     assert(opts.languageCode, 'languageCode is required')
 
     Object.assign(this, { ...opts })
-    this.relativePath = slash(this.relativePath)
-    this.fullPath = slash(path.join(this.basePath, this.relativePath))
-    this.raw = fs.readFileSync(this.fullPath, 'utf8')
 
     // TODO remove this when crowdin-support issue 66 has been resolved
     if (this.languageCode !== 'en' && this.raw.includes(': verdadero')) {

diff --git a/lib/pages.js b/lib/pages.js
@@ -2,42 +2,50 @@ const path = require('path')
 const walk = require('walk-sync').entries
 const Page = require('./page')
 const languages = require('./languages')
-const fs = require('fs')
+const { mapLimit, filterLimit } = require('async')
+const FILE_READ_LIMIT = 500
 
 async function loadPageList () {
   const pageList = []
 
   // load english pages
   const englishPath = path.join(__dirname, '..', languages.en.dir, 'content')
-  const englishPages = walk(englishPath)
-    .filter(({ relativePath }) => {
-      return relativePath.endsWith('.md') &&
-      !relativePath.includes('README')
-    })
-    .map(fileData => new Page({ ...fileData, languageCode: languages.en.code }))
-
+  const englishPaths = walk(englishPath)
+    .filter(({ relativePath }) =>
+      relativePath.endsWith('.md') && !relativePath.includes('README')
+    )
+  const englishPages = await mapLimit(
+    englishPaths,
+    FILE_READ_LIMIT,
+    async fileData => await Page.init({ ...fileData, languageCode: languages.en.code })
+  )
   pageList.push(...englishPages)
 
   // load matching pages in other languages
-  for (const page of englishPages) {
-    for (const language of Object.values(languages)) {
-      if (language.code === 'en') continue
-
+  let localizedPaths = Object.values(languages)
+    .filter(({ code }) => code !== 'en')
+    .map(language => {
       const basePath = path.join(__dirname, '..', language.dir, 'content')
-      const localizedPath = path.join(basePath, page.relativePath)
-      try {
-        fs.statSync(localizedPath)
-      } catch (_) {
-        continue
-      }
-
-      pageList.push(new Page({
-        relativePath: page.relativePath,
+      return englishPages.map(page => ({
         basePath,
+        relativePath: page.relativePath,
+        localizedPath: path.join(basePath, page.relativePath),
         languageCode: language.code
       }))
-    }
-  }
+    })
+    .flat()
+  localizedPaths = await filterLimit(
+    localizedPaths,
+    FILE_READ_LIMIT,
+    async ({ localizedPath }) => Page.exists(localizedPath)
+  )
+  const localizedPages = await mapLimit(
+    localizedPaths,
+    FILE_READ_LIMIT,
+    async ({ basePath, relativePath, languageCode }) =>
+      await Page.init({ basePath, relativePath, languageCode })
+  )
+  pageList.push(...localizedPages)
 
   return pageList
 }

diff --git a/tests/browser/browser.js b/tests/browser/browser.js
@@ -3,13 +3,17 @@ const sleep = require('await-sleep')
 const querystring = require('querystring')
 
 describe('homepage', () => {
+  jest.setTimeout(60 * 1000)
+
   test('should be titled "GitHub Documentation"', async () => {
     await page.goto('http://localhost:4001')
     await expect(page.title()).resolves.toMatch('GitHub Documentation')
   })
 })
 
 describe('algolia browser search', () => {
+  jest.setTimeout(60 * 1000)
+
   it('works on the homepage', async () => {
     await page.goto('http://localhost:4001/en')
     await page.click('#search-input-container input[type="search"]')

diff --git a/tests/content/crowdin-config.js b/tests/content/crowdin-config.js
@@ -4,6 +4,8 @@ const ignoredPagePaths = config.files[0].ignore
 const ignoredDataPaths = config.files[2].ignore
 
 describe('crowdin.yml config file', () => {
+  jest.setTimeout(60 * 1000)
+
   let pages
   beforeAll(async (done) => {
     pages = await loadPages()

diff --git a/tests/content/site-data-references.js b/tests/content/site-data-references.js
@@ -7,6 +7,8 @@ const fs = require('fs').promises
 const path = require('path')
 
 describe('data references', () => {
+  jest.setTimeout(60 * 1000)
+
   let data, pages
 
   beforeAll(async (done) => {

diff --git a/tests/routing/redirects.js b/tests/routing/redirects.js
@@ -17,17 +17,17 @@ describe('redirects', () => {
     done()
   })
 
-  test('page.redirects is an array', () => {
-    const page = new Page({
+  test('page.redirects is an array', async () => {
+    const page = await Page.init({
       relativePath: 'github/collaborating-with-issues-and-pull-requests/about-branches.md',
       basePath: path.join(__dirname, '../../content'),
       languageCode: 'en'
     })
     expect(isPlainObject(page.redirects)).toBe(true)
   })
 
-  test('dotcom homepage page.redirects', () => {
-    const page = new Page({
+  test('dotcom homepage page.redirects', async () => {
+    const page = await Page.init({
       relativePath: 'github/index.md',
       basePath: path.join(__dirname, '../../content'),
       languageCode: 'en'
@@ -41,7 +41,7 @@ describe('redirects', () => {
   })
 
   test('converts single `redirect_from` strings values into arrays', async () => {
-    const page = new Page({
+    const page = await Page.init({
       relativePath: 'github/collaborating-with-issues-and-pull-requests/about-conversations-on-github.md',
       basePath: path.join(__dirname, '../../content'),
       languageCode: 'en'

diff --git a/tests/unit/find-page.js b/tests/unit/find-page.js
@@ -8,7 +8,7 @@ describe('find page', () => {
   jest.setTimeout(1000 * 1000)
 
   test('falls back to the English page if it can\'t find a localized page', async () => {
-    const page = new Page({
+    const page = await Page.init({
       relativePath: 'page-that-does-not-exist-in-translations-dir.md',
       basePath: path.join(__dirname, '../fixtures'),
       languageCode: 'en'
@@ -24,7 +24,7 @@ describe('find page', () => {
   })
 
   test('follows redirects', async () => {
-    const page = new Page({
+    const page = await Page.init({
       relativePath: 'page-with-redirects.md',
       basePath: path.join(__dirname, '../fixtures'),
       languageCode: 'en'

diff --git a/tests/unit/liquid-helpers.js b/tests/unit/liquid-helpers.js
@@ -5,6 +5,8 @@ const { set } = require('lodash')
 const nonEnterpriseDefaultVersion = require('../../lib/non-enterprise-default-version')
 
 describe('liquid helper tags', () => {
+  jest.setTimeout(60 * 1000)
+
   const context = {}
   let pageMap
   beforeAll(async (done) => {