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 pygments styles #1211

Closed
isidentical opened this issue Nov 24, 2021 · 6 comments · Fixed by #1221
Closed

Lazily load pygments styles #1211

isidentical opened this issue Nov 24, 2021 · 6 comments · Fixed by #1221
Assignees
Milestone

Comments

@isidentical
Copy link
Contributor

We currently try to retrieve all pygments styles at once, but this operation is very slow due to the import of pkg_resources on the pygments side.
https://github.com/httpie/httpie/blob/cfcd7413d1afbabc5ab3c88a892851335be4944a/httpie/output/formatters/colors.py#L31

We should load these styles lazily when a user specifies --style explicitly.

@isidentical isidentical self-assigned this Nov 24, 2021
@isidentical isidentical added this to the HTTPie 3.0 milestone Nov 24, 2021
@isidentical
Copy link
Contributor Author

isidentical commented Nov 24, 2021

Command: hyperfine 'http --version'

normal lazy loading factor
Environment 1 238.2 ms 185.4 ms 1.30X (30%)
Environment 2 383.3 ms 240.4 ms 1.58X (58%)

Since the actual load we are removing is pkg_resources import, the speed-up factor is very dependant on how complex the current python environment you are within. The Environment 1 is simply a fresh python docker image with only httpie installed. The Environment 2 is my local environment with over 300+ python packages.

@isidentical
Copy link
Contributor Author

(I'll send a patch after #1200)

@jkbrzt
Copy link
Member

jkbrzt commented Nov 24, 2021

Nice!

Could we do some quick lookup even if the user specifies --style? Many users probably have --style in their config file.

The full list is probably only needed for --help output. But also even then we only need the names.

@isidentical
Copy link
Contributor Author

isidentical commented Nov 24, 2021

Could we do some quick lookup even if the user specifies --style? Many users probably have --style in their config file.

If users specify --style, they will load the whole pygments (including all of it's plugins). And it will be like the original, so we will error out if the style is not available etc.

The full list is probably only needed for --help output. But also even then we only need the names.

There is actually no cost distinction between the names of the styles and the loading of actual lexers, since most of the time is spent on the pkg_resources import. With this patch, we will only load the pygments on the following 3 occasions:

  • When you pass a valid --style (to verify it exists)
  • When you pass an invalid --style (to print all the choices)
  • To format the help message when run with --help (this is the trickiest, since --help needs to be formatted lazily but there is no easy way to do so).

@jkbrzt
Copy link
Member

jkbrzt commented Dec 1, 2021

When you pass a valid --style (to verify it exists)

Couldn’t we use a different method for checking if it exists (e.g. checking if a module exists first, and only then loading the styles if not)? If I use a custom --style (via config, for example), I won’t be able to benefit from this optimization.

@isidentical
Copy link
Contributor Author

Couldn’t we use a different method for checking if it exists (e.g. checking if a module exists first, and only then loading the styles if not)?

We could introduce layers (e.g check if this is a built-in httpie style > check if this is a built-in pygments style > check if this is coming from a pygments plugin), but I think that would be a bit complicated.

Another possibility is asking pygments maintainers whether they would accept a patch that migrates from pkg_resources to importlib_metadata (a much much faster version for iterating over entry points), if the use case where a --style is also important to us.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants