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

Avoid repeatedly invoking pip show in Python lang host #7819

Closed
wants to merge 1 commit into from

Conversation

benesch
Copy link
Contributor

@benesch benesch commented Aug 22, 2021

Description

The Python language host invokes pip show for each Pulumi package in a
project at startup. But pip show is quite slow in large projects: it
takes 2+ seconds per invocation in a project at @MaterializeInc.

pip show is invoked to compute the installed location of each plugin
package. But it turns out pip list takes a -v flag that can supply
this information in one shot, avoiding the need to ever invoke pip show.

This patch shaves about 20s off our boot time for pulumi up.

(There's probably a separate bug in Pip that causes pip show to be so
slow, because pip list is an order of magnitude faster and does a lot
more work, but I didn't bother tracking that down.)

Checklist

  • I have added tests that prove my fix is effective or that my feature works

Seems hard to write a test for. If the existing tests pass then this at least didn't break anything.

Are perf improvements like this usually noted in the changelog?

  • Yes, there are changes in this PR that warrants bumping the Pulumi Service API version

Nope.

@github-actions
Copy link

PR is now waiting for a maintainer to run the acceptance tests.

Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR

Further commands available:

  • /run-codegen - used to test the Pull Request against downstream codegen
  • /run-docs-gen - used to test the Pull Request against documentation generation

@github-actions
Copy link

PR is now waiting for a maintainer to run the acceptance tests.

Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR

Further commands available:

  • /run-codegen - used to test the Pull Request against downstream codegen
  • /run-docs-gen - used to test the Pull Request against documentation generation

The Python language host invokes `pip show` for each Pulumi package in a
project at startup. But `pip show` is quite slow in large projects: it
takes 2+ seconds per invocation in a project at @MaterializeInc.

`pip show` is invoked to compute the installed location of each plugin
package. But it turns out `pip list` takes a `-v` flag that can supply
this information in one shot, avoiding the need to ever invoke `pip
show`.

This patch shaves about 20s off our boot time for `pulumi up`.

(There's probably a separate bug in Pip that causes `pip show` to be so
slow, because `pip list` is an order of magnitude faster and does a lot
more work, but I didn't bother tracking that down.)
@github-actions
Copy link

PR is now waiting for a maintainer to run the acceptance tests.

Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR

Further commands available:

  • /run-codegen - used to test the Pull Request against downstream codegen
  • /run-docs-gen - used to test the Pull Request against documentation generation

@t0yv0 t0yv0 self-requested a review August 23, 2021 14:45
@t0yv0
Copy link
Member

t0yv0 commented Aug 23, 2021

/run-acceptance-tests

@github-actions
Copy link

Please view the results of the PR Build + Acceptance Tests Run Here

@emiliza emiliza added this to the 0.61 milestone Aug 23, 2021
@t0yv0
Copy link
Member

t0yv0 commented Aug 23, 2021

There are some false test failures (lint is currently broken on master, I'm fixing separately), but there is at least one test failure that is pertinent to this PR:

=== FAIL: go/auto TestRuntimeErrorPython (7.26s)
    errors_test.go:396: 
        	Error Trace:	errors_test.go:396
        	Error:      	Should be true
        	Test:       	TestRuntimeErrorPython
        	Messages:   	failed to run update: exit status 255
        	            	code: 255
        	            	stdout: Updating (pulumi-test/int_test37968375)
        	            	
        	            	View Live: https://app.pulumi.com/pulumi-test/runtime_error/int_test37968375/updates/1
        	            	
        	            	 
        	            	
        	            	stderr: error: failed to discover plugin requirements: parsing `python -m pip list -v --format json` output: invalid character '1' after top-level value
        	            	

DONE 47 tests, 3 skipped, 1 failure in 354.751s

I'll investigate further tomorrow, hopefully a simple fix.

@t0yv0
Copy link
Member

t0yv0 commented Aug 25, 2021

Merged via #7831

Thank you very very much. This is a fix that makes me personally quite excited.

@t0yv0 t0yv0 closed this Aug 25, 2021
@benesch
Copy link
Contributor Author

benesch commented Aug 25, 2021

Thanks very much for fixing that test failure! The non-JSON trailer was a great catch. We're very excited for the impending speedup to our pulumi ups at @MaterializeInc!

@benesch benesch deleted the no-pip-show branch August 25, 2021 16:34
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