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

Improve profile builder performance #437

Merged
merged 1 commit into from
Jun 29, 2023

Conversation

Goose97
Copy link
Contributor

@Goose97 Goose97 commented Jun 29, 2023

Follow up on this PR #435.

Currently, it took roughly 22 seconds to load my 1.3GB file. After inspecting the profiler, there's a large chunk of time spending in Frame.getOrInsert. I figure we can reduce the number of invocations by half. It reduces the load time to roughly 18 seconds.I also tested with a smaller file (~350MB), and it show similar gains, about 15-20%

I know this is not a proper way to perform benchmarking, and also I'm clueless about the impact on other file formats/sizes. So I'll leave the judgement to you

Profiling shows that a significant amount of time is spent in Frame.getOrInsert. This PR improves that by reducing the amount of calls to said function
@coveralls
Copy link

Coverage Status

coverage: 42.952%. remained the same when pulling da4c0b2 on Goose97:improve-profile-builder-performance into 984bf12 on jlfwong:main.

@jlfwong
Copy link
Owner

jlfwong commented Jun 29, 2023

Wonderful! Nice tactical performance fix

@jlfwong jlfwong merged commit f62519a into jlfwong:main Jun 29, 2023
8 checks passed
jackerghan pushed a commit to jackerghan/speedscope that referenced this pull request Jul 28, 2023
Follow up on this PR jlfwong#435. 

Currently, it took roughly 22 seconds to load my 1.3GB file. After inspecting the profiler, there's a large chunk of time spending in Frame.getOrInsert. I figure we can reduce the number of invocations by half. It reduces the load time to roughly 18 seconds.I also tested with a smaller file (~350MB), and it show similar gains, about 15-20%
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

3 participants