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.

Sign up for GitHub

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

Jump to bottom

Investigate why V2 test runner is slower than V1 #7795

Closed
Eric-Arellano opened this issue May 23, 2019 · 3 comments
Closed

Investigate why V2 test runner is slower than V1 #7795

Eric-Arellano opened this issue May 23, 2019 · 3 comments
Assignees
@Eric-Arellano

Comments

@Eric-Arellano
Copy link
Contributor

When changing our CI from V1 to V2 for unit tests, performance goes from about 25 minutes to 40 minutes. Why is this?

Even though remoting will likely make the end result be significantly faster, non-remoting should not be taking this much longer and we are likely missing several optimizations.

See #7724 (comment) and the comments below it for some hypotheses.
@Eric-Arellano Eric-Arellano mentioned this issue May 23, 2019
Merged
@stuhood
Copy link
Member

stuhood commented May 31, 2019

Relates to #6898. Adding support for local process caching would effectively eliminate the resolve step run over run (without pantsd: it's already eliminated with pantsd), which is itself significant. It would also reduce the number of tests that actually run.

@Eric-Arellano Eric-Arellano mentioned this issue Jun 1, 2019
Merged
Eric-Arellano added a commit that referenced this issue Jun 3, 2019
@Eric-Arellano
Bump CI unit test timeout for less flaky runs (#7831)
d72fb81 
When using V2, unit tests take too long to run so we have to use `travis_wait`. We set the wait time too short, and have a few times had timeouts.

Note that a better solution would be to figure out why V2 is slower in the first place via #7795, followed by implementing local caching via #6898.
@Eric-Arellano Eric-Arellano self-assigned this Jun 3, 2019
@Eric-Arellano
Copy link
Contributor Author

Indeed, ~75% of the time is spent on the requirements PEX (7.3 / 9.3 seconds). See this gist for the diff used to investigate and different timing results.

So, the best solution is to speed up the requirements PEX, e.g. via local caching (#6898).

@Eric-Arellano
Copy link
Contributor Author

Closing because we have some good answers.

@Eric-Arellano Eric-Arellano mentioned this issue Jun 7, 2019
Merged
Eric-Arellano added a commit that referenced this issue Jun 7, 2019
@Eric-Arellano
Extract out resolve_requirements V2 rule for creating PEXes with re…
530a06c 
…quirements (#7846)

### Problem
`python_test_runner.py` does too much right now. 

One of its main pieces of logic that should be factored out is creating a PEX with the desired requirements, entry point, and interpreter constraints. This will presumably be useful for other V2 rules, so should not be coupled to the Pytest runner.

Also, it was found in #7795 that this requirements PEX is causing ~75% of the performance issues for the V2 Pytest runner. Having it extracted out into its own rule will make it easier to develop, test, and optimize.

### Solution
Create a generic rule, along with dedicated tests.
@Eric-Arellano Eric-Arellano mentioned this issue Nov 22, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Eric-Arellano Eric-Arellano

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@stuhood @Eric-Arellano