Skip to content

Decrease start-up time of editable-installed entry points on newer versions of Python#2194

Merged
jaraco merged 3 commits into
pypa:masterfrom
ofek:fast
Jun 15, 2020
Merged

Decrease start-up time of editable-installed entry points on newer versions of Python#2194
jaraco merged 3 commits into
pypa:masterfrom
ofek:fast

Conversation

@ofek

@ofek ofek commented Jun 14, 2020

Copy link
Copy Markdown
Contributor

Summary of changes

When importlib.metadata is present, the generated scripts will use that rather than the significantly slower pkg_resources.

Closes #510 completely as editable installs are the final affected part (that is actionable) brought up in that issue and what I've been experiencing the last 2 weeks (it's indeed annoying)

The issue is locked, so here are some pings: @jaraco @scopatz @ninjaaron @untitaker @asottile @pganssle @gaborbernat @pfmoore @pradyunsg

Benchmark

Before and after, using hyperfine:

Capture

Pull Request Checklist

  • Changes have tests
  • News fragment added in changelog.d. See documentation for details

Comment thread setuptools/tests/test_easy_install.py Outdated

if __name__ == '__main__':
sys.argv[0] = re.sub(r'(-script\.pyw?|\.exe)?$', '', sys.argv[0])
for entry_point in distribution(%(spec)r).entry_points:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

should this use entry_points()['console_scripts'] perhaps instead?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Wouldn't that enumerate every script for every package?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

iirc distribution already enumerates every package but I might be misremembering

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I was surprised this interface isn't nicer :/. Yes, I believe this usage is correct. Probably importlib-metadata should provide some helpers to make this usage simpler, though that won't help for this implementation.

Comment thread setuptools/command/easy_install.py Outdated
load_entry_point(%(spec)r, %(group)r, %(name)r)()
)
""").lstrip()
if sys.version_info >= (3, 8):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm trying to imagine a way this could rely also on importlib_metadata and thus eliminate the dependency on pkg_resources. I dislike that the code has to be forked to support the legacy behavior (and until Python 3.7 support is dropped). Perhaps that can't be solved now.

load_entry_point(%(spec)r, %(group)r, %(name)r)()
)
""").lstrip()
if sys.version_info >= (3, 8):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I also wonder if it would make sense to unify these two scripts into one which tries importlib-metadata then falls back to pkg_resources. That approach would avoid the fork and would also avoid the suppressed linter errors.

load_entry_point('spec', 'console_scripts', 'name')()
)
""") # noqa: E501
if sys.version_info >= (3, 8):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'll plan to consolidate this fork later... and just make some assertions about the result that are common to both behaviors.

@jaraco jaraco left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

My comments are largely for edification. No action is necessary. Let's adopt this and iterate on it. Thanks for the contrib.

@jaraco jaraco merged commit 9befd91 into pypa:master Jun 15, 2020
@ofek ofek deleted the fast branch June 15, 2020 02:24
@ofek

ofek commented Jun 15, 2020

Copy link
Copy Markdown
Contributor Author

Thank you!

@ofek ofek mentioned this pull request Jun 15, 2020
2 tasks
@ofek

ofek commented Jun 15, 2020

Copy link
Copy Markdown
Contributor Author

Needs small fix #2196

archlinux-github pushed a commit to archlinux/svntogit-community that referenced this pull request Feb 10, 2021
pypa/setuptools#2194

git-svn-id: file:///srv/repos/svn-community/svn@852053 9fca08f4-af9d-4005-b8df-a31f2cc04f65
archlinux-github pushed a commit to archlinux/svntogit-community that referenced this pull request Feb 10, 2021
pypa/setuptools#2194


git-svn-id: file:///srv/repos/svn-community/svn@852053 9fca08f4-af9d-4005-b8df-a31f2cc04f65
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.

Avoid full path enumeration on import of setuptools or pkg_resources?

3 participants