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

Investigate why V2 test runner is slower than V1 #7795

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

Comments

Projects
None yet
2 participants
@Eric-Arellano
Copy link
Contributor

commented May 23, 2019

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.

@stuhood

This comment has been minimized.

Copy link
Member

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 added a commit that referenced this issue Jun 3, 2019

Bump CI unit test timeout for less flaky runs (#7831)
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

This comment has been minimized.

Copy link
Contributor Author

commented Jun 4, 2019

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

This comment has been minimized.

Copy link
Contributor Author

commented Jun 4, 2019

Closing because we have some good answers.

Eric-Arellano added a commit that referenced this issue Jun 7, 2019

Extract out `resolve_requirements` V2 rule for creating PEXes with re…
…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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.