Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf(v2): more efficient hot reload & consistent filegen #1950

Merged
merged 3 commits into from
Nov 8, 2019

Conversation

endiliey
Copy link
Contributor

@endiliey endiliey commented Nov 8, 2019

Motivation

More efficient hot reload & consistent generated file

Note that I set clientLogLevel to info in devserver config.

Previously, if we start development server and go to home page. We can go to src/pages/index.js and just 'save' without changing anything

many are recompiled

GIF Preview
Nothing changed but lot of recompile

Note that there are several files that got disposed by WDS. This is because our generated files (although we have caching in

const fileHash = new Map();
export async function generate(
generatedFilesDir: string,
file: string,
content: any,
): Promise<void> {
const filepath = path.join(generatedFilesDir, file);
const lastHash = fileHash.get(filepath);
const currentHash = createHash('md5')
.update(content)
.digest('hex');
if (lastHash !== currentHash) {
await fs.ensureDir(path.dirname(filepath));
await fs.writeFile(filepath, content);
fileHash.set(filepath, currentHash);
}
}
), is re-written due to different object keys ordering.

Explanation

{
'a': 'foo',
'b': 'bar',
}

Is considered different from

{
'b': 'bar',
'a': 'foo',
}

The generated files can be different because of async nature and keys ordering. We should always provide consistent file generation.

Others:

Have you read the Contributing Guidelines on pull requests?

yes

Test Plan

After this PR
consistent

GIF preview
Nothing changed

I tried editing the src/pages/index.js for real and its the right behavior
correct behavior

Index.js is detected to have changed, so it is disposed. Registry need to be dispsoed because index.js has changed, and so on.

Extra: Notice the hot reload performance is much faster :)

@endiliey endiliey requested a review from yangshun as a code owner November 8, 2019 09:09
@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Nov 8, 2019
@endiliey endiliey changed the title Endi/reloadconsistent perf/fix(v2): more efficient hot reload & consistent filegen Nov 8, 2019
Copy link
Contributor

@yangshun yangshun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow what an optimization. Maybe you could tweet/blog about this.

@docusaurus-bot
Copy link
Contributor

Deploy preview for docusaurus-2 ready!

Built with commit 32af3c1

https://deploy-preview-1950--docusaurus-2.netlify.com

@yangshun yangshun merged commit e04c8f1 into master Nov 8, 2019
@docusaurus-bot
Copy link
Contributor

Deploy preview for docusaurus-preview ready!

Built with commit 32af3c1

https://deploy-preview-1950--docusaurus-preview.netlify.com

@@ -18,9 +18,6 @@ import {posixPath} from '@docusaurus/utils';
const createFakeActions = (routeConfigs: RouteConfig[], contentDir) => {
return {
addRoute: (config: RouteConfig) => {
config.routes.sort((a, b) =>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously had to sort it in test because snapshot will always fail previously due to inconsistent routes (async nature). Now no longer need to do that in test

@@ -202,7 +207,7 @@ export default function pluginContentDocs(
docsDir,
docsSidebars,
sourceToPermalink,
permalinkToSidebar,
permalinkToSidebar: objectWithKeySorted(permalinkToSidebar),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had to do this because otherwise docsBaseMetadata (the root file generated) will always change without utilizing cache

c: 'bar',
a: 'baz',
};
expect(objectWithKeySorted(obj2)).toMatchInlineSnapshot(`
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even snapshot is consistent now in ordering😃

@endiliey
Copy link
Contributor Author

endiliey commented Nov 8, 2019

it got merged before i can even comment 😭 .

Actually found this when stress testing 1,000 docs. My hot reload went down from 10s to 1-2s due to this optimization lol

1026 docs

Editing pages only took 600ms as usual
1026 files, pages reloaded

Seems we can handle 1k+ docs for now
~45s for cold startup

~20s after cache
after cache 20s

@endiliey endiliey deleted the endi/reloadconsistent branch November 8, 2019 09:33
@yangshun
Copy link
Contributor

yangshun commented Nov 8, 2019

Sorry, I was too excited to merge this. This is such a crazy optimization. We are one step closer towards the scalability required for versioning!

@endiliey
Copy link
Contributor Author

endiliey commented Nov 8, 2019

On production, removing

showLastUpdateTime and showLastUpdateAuthor saved 80s of nodejs time 😭 for 1026 docs lol
Thankfully we use fake data for dev :)

@endiliey endiliey added pr: bug fix This PR fixes a bug in a past release. pr: performance This PR does not add a new behavior, but existing behaviors will be more memory- / time-efficient. and removed pr: bug fix This PR fixes a bug in a past release. labels Nov 8, 2019
@endiliey endiliey changed the title perf/fix(v2): more efficient hot reload & consistent filegen perf(v2): more efficient hot reload & consistent filegen Nov 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Signed Facebook CLA pr: performance This PR does not add a new behavior, but existing behaviors will be more memory- / time-efficient.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants