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

[pantsd] Don't initialize a scheduler for pantsd lifecycle checks. #5624

Merged
merged 3 commits into from Mar 27, 2018

Conversation

Projects
None yet
3 participants
@kwlzn
Copy link
Member

kwlzn commented Mar 23, 2018

good for a ~40% speedup in lifecycle overhead for a 25 no-op run test case:

before:

[omerta pants (master)]$ ./pants -q clean-all; time (for i in $(seq 1 25); do PANTS_ENABLE_PANTSD=True ./pants >/dev/null 2>&1; done)
real    0m59.543s
user    0m23.667s
sys     0m12.274s

after:

[omerta pants (kwlzn/5433)]$ ./pants -q clean-all; time (for i in $(seq 1 25); do PANTS_ENABLE_PANTSD=True ./pants >/dev/null 2>&1; done)
real    0m42.530s
user    0m14.582s
sys     0m8.409s

clean-all/kill-pantsd with and without the daemon is also snappier as well.

Fixes #5433

@kwlzn kwlzn requested review from stuhood and illicitonion Mar 23, 2018

@kwlzn kwlzn force-pushed the twitter:kwlzn/5433 branch 2 times, most recently from e2c944e to d28745c Mar 23, 2018

@kwlzn kwlzn changed the title [pantsd] Don't initialize a scheduler for pantsd lifecycle check. [pantsd] Don't initialize a scheduler for pantsd lifecycle. Mar 24, 2018

@kwlzn kwlzn force-pushed the twitter:kwlzn/5433 branch from 98d4248 to b5e799c Mar 24, 2018

@kwlzn kwlzn changed the title [pantsd] Don't initialize a scheduler for pantsd lifecycle. [pantsd] Don't initialize a scheduler for pantsd lifecycle checks. Mar 24, 2018

@kwlzn kwlzn force-pushed the twitter:kwlzn/5433 branch 2 times, most recently from a6649a4 to 59cbe63 Mar 26, 2018

@illicitonion
Copy link
Contributor

illicitonion left a comment

This seems reasonable, but I don't have enough context to know whether this is missing any important cases.

@stuhood
Copy link
Member

stuhood left a comment

Thanks Kris.

"""
:param Options bootstrap_options: The bootstrap options, if available.
:param bool full_init: Whether or not to fully initialize the engine.

This comment has been minimized.

@stuhood

stuhood Mar 26, 2018

Member

"full" isn't really defined anywhere, so it would be good to either: 1) describe what "full" means and what will not work if !full, 2) split this into two methods that accomplish the two halves of the initialization (ie, the first returns a result that is optionally passed to the second).

@kwlzn kwlzn force-pushed the twitter:kwlzn/5433 branch from 59cbe63 to 8bd346c Mar 27, 2018

@kwlzn kwlzn merged commit 50046c7 into pantsbuild:master Mar 27, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment