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

Fixed performance issue with JSON files #40

Merged
merged 1 commit into from Nov 25, 2022

Conversation

LennardF1989
Copy link
Member

@LennardF1989 LennardF1989 commented Nov 23, 2022

Makes changes to the build process to handle imported JSON files differently. This significantly increases startup time (favorable for unit testing), as well as pretty significantly decreasing the final build size when building (going from 27 mb to just 7 mb).

Changes for speed: The JSON imports will be loaded as strings instead of JavaScript objects.
Changes for build size: The JSON imports will be reserialized without indentation.

Draft because I want to verify if things still work as intended (they should, though).

Comment on lines 30 to 45
const jsonAsCompressedTextPlugin = {
name: "jsonAsCompressedTextPlugin",

setup(build) {
let fs = require("fs")

build.onLoad({ filter: /\.json$/ }, async (args) => {
const text = await fs.promises.readFile(args.path)

return {
contents: JSON.stringify(JSON.parse(text)),
loader: "text",
}
})
},
}
Copy link
Member

Choose a reason for hiding this comment

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

I think this can just use the loader option instead of making this a plugin.

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope, that was my first attempt and it will only treat it as text*, and not actually remove indentation like I do now. The JSON loader will turn it into an object, so this is the only way to do it and also "compress" the actual resulting string :)

* loaders is also a fixed list of available loaders.

Copy link
Member Author

Choose a reason for hiding this comment

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

let fs = require("fs") I want to change this to const though. Using an import doesn't work do to the way esbuild works with plugins.

@LennardF1989 LennardF1989 force-pushed the json-performance branch 2 times, most recently from 3cad4f4 to 55787b8 Compare November 25, 2022 19:50
@LennardF1989 LennardF1989 marked this pull request as ready for review November 25, 2022 19:55
@LennardF1989
Copy link
Member Author

LennardF1989 commented Nov 25, 2022

Tested in-game:

  • by replacing chunk0.js in latest release
  • by dev-start

Startup time significantly reduced. Build size significantly reduced (6.5MB after optimizing).

EDIT: Also, my test-branch is reliant on this fix, which is also ready for further extension by taking it into the core.

@RDIL RDIL merged commit c372412 into thepeacockproject:v5 Nov 25, 2022
@LennardF1989 LennardF1989 deleted the json-performance branch November 25, 2022 20:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants