-
Notifications
You must be signed in to change notification settings - Fork 62
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
Optimizations for large builds #37
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
On a snapshot of editor build: --stop: 4.8s (5834.4MB alloc), --analyze: 9.9s (7261.7MB alloc).
…ing and name deduplication happens during --stop, not during --analyze. On a snapshot of unity editor build: --stop: 4.8s 5834MB -> 9.5s 7260MB --analyze: 9.9s 7262MB -> 4.5s 1270MB data file: 1.01GB -> 268MB
…of memory too: On a snapshot of unity editor build: --stop: 9.5s 7260MB -> 9.5s 6968MB --analyze: 4.5s 1270MB (unchanged) data file: 268MB (unchanged)
On a snapshot of unity editor build: --stop: 9.5s 6968MB -> 9.3s 6975MB
…tell_hash_map (https://probablydance.com/2018/05/28/a-new-fast-hash-table-in-response-to-googles-new-fast-hash-table/) On a snapshot of unity editor build: --stop: 9.3s 6975MB -> 8.2s 6984MB
On a snapshot of unity editor build: --stop: 8.2s 6984MB -> 7.8s 6071MB
On a snapshot of unity editor build: --stop: 7.8s 6071MB -> 6.5s 6071MB
On a snapshot of unity editor build: --analyze: 4.5s 1270MB -> 3.4s 1519MB
On a snapshot of unity editor build: --analyze: 3.4s 1519MB -> 3.3s 1437MB
ben-craig
reviewed
Mar 1, 2020
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
aras-p doesn't optimize runtime code.
aras-p doesn't optimize build throughput.
aras-p optimizes the build throughput profiler.
I'm now expecting performance patches for whatever perf visualizer you use.
…ry usage on large codebases is a problem, and is not thread safe either. On a snapshot of unity editor build: --stop: 6.5s 6071MB -> 7.0s 493MB --analyze: 3.3s 1437MB -> 4.7s 779MB
…hash function. On a snapshot of unity editor build: --stop: 7.0s 493MB -> 5.4s 466MB; BuildEventsParser::NameToIndex 1.47s -> 960ms
…g analysis. On a snapshot of unity editor build: --stop: 5.4s 466MB -> 5.4s 342MB --analyze: 4.7s 779MB -> 4.1s 750MB
On a snapshot of unity editor build: --stop: 5.4s 342MB -> 5.1s 477MB
On a snapshot of unity editor build (MacBookPro 2018, 6c/12t): --stop: 5.1s -> 2.4s
…dly loading wrong files
…alyze On a snapshot of unity editor build: --stop: 2.4s -> 2.3s, 368MB --analyze: 4.4s 750MB -> 4.4s, 673MB
On a snapshot of unity editor build: --stop: 2.3s 368MB -> 2.4s, 348MB --analyze: 4.4s 673MB -> 3.6s 641MB
On a snapshot of unity editor build: --stop: unchanged at 2.4s 348MB --analyze: 3.6s 641MB -> 2.9s 578MB
…psedTemplateOpt), only actually process templated functions. This makes it faster (skips over many more functions without attempting to collapse the template name), and also "fixes" output where function sets are actual sets and not random slow individual functions. On a snapshot of unity editor build: --stop: unchanged at 2.4s 348MB --analyze: 2.9s 578MB -> 2.7s 505MB
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Redo how the data is gathered & stored to better scale for large builds (ref #10, #30).
TLDR: 3x faster, 15x lower memory usage, 4x smaller file size.
Previously,
--stop
was just smashing all input JSON files together, and--analyze
was parsing that huge JSON file, and performing the analysis. The memory allocator used was a simple "bump a pointer, never free" allocator. On a decent size project (e.g. Unity editor build), that was taking 4.8sec / 5.8GB for stop, and 9.9sec / 7.3GB for analyze (so total 14.7sec, and max 7.3GB memory usage). The resulting file was 1.01GB size.Now:
--stop
step into the build events data structures (which includes string/name deduplication), and as a result just store the binary data structure file.--analyze
reads that binary file and does no JSON parsing at all now.--stop
part that parses all the JSON files, using enkiTS task scheduler.sajson
JSON parser with simdjson.On the same Unity editor build: 2.4sec / 0.35GB for stop, and 2.7sec / 0.51GB for analyze (so total 5.1sec, and max 0.5GB memory usage). The resulting file is 280MB size.
However the data files produced by
--stop
and used by--analyze
are no longer compatible with previous versions (previous were just JSON, now custom binary).