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

Lazily load python package mappings #51

Merged
merged 1 commit into from
Sep 1, 2020
Merged

Conversation

cbrewster
Copy link
Member

These maps were always being loaded into memory on startup, causing significant overhead for every UPM command.

Before:
Messages Image(1355001053)

After:
Screen Shot 2020-09-01 at 11 18 07 AM

These were always being loaded into memory, causing significant overhead for every UPM command
@replbot replbot added boop and removed preboop labels Sep 1, 2020
@replbot
Copy link
Member

replbot commented Sep 1, 2020

Good work, this PRs short and easy to review! Promoting to bop.

@replbot replbot added the bop label Sep 1, 2020
Copy link

@turbio turbio left a comment

Choose a reason for hiding this comment

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

easy peasy! 🥬

Pretty interesting, my working assumption was the package map would end up in .data + .rodata and be loaded on demand. Any idea why that didn't actually work? I'm guessing go's initialization does some extra magic and hits the map early???

@replbot replbot removed the boop label Sep 1, 2020
@replbot
Copy link
Member

replbot commented Sep 1, 2020

unbooping: approved

@cbrewster cbrewster merged commit 88b820a into master Sep 1, 2020
@cbrewster
Copy link
Member Author

Yeah I was pretty surprised when I saw the map show up in the profile when running upm help, must be something with go's initialization model.

@amasad
Copy link
Member

amasad commented Sep 1, 2020

nice

@cbrewster cbrewster deleted the cb-lazy-py-package-map branch September 1, 2020 18:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants