This repository has been archived by the owner on Jan 13, 2022. It is now read-only.
Add new profling options for Facebook/hhvm #28
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| $this->tc_toptrans = array_key_exists('dump-top-trans', $o); | ||
| $this->tc_topfuncs = array_key_exists('dump-top-funcs', $o); | ||
| $this->pcredump = array_key_exists('dump-pcre-cache', $o); | ||
| $this->profBC = array_key_exists('profBC', $o); |
There was a problem hiding this comment.
FB standard is:
- locals are under_scored
- members are camelCased
Hopefully I was consistent, but these new ones aren't even with each other.
paulbiss
force-pushed
the
perf-options
branch
2 times, most recently
from
d70b1ba to
41ffdb8
Compare
|
|
||
| invariant(is_dir($sourceRoot), 'Could not find valid source root'); | ||
|
|
||
| $files = shell_exec("cd $sourceRoot && find . -type f"); |
There was a problem hiding this comment.
recursiveiteratoriterator + recursivedirectoryiterator would be nice
|
Given how many shell_exec() this adds, probably worth adding a helper that takes string $cmd, Vector args. |
|
Can you force-push a rebase+squash, ideally add the shell_exec() function, then I'll merge this? |
|
|
||
| // Pause once benchmarking is complete to allow for manual inspection of the | ||
| // HHVM or PHP process. | ||
| public bool $waitAtEnd = false; |
fredemmott
added a commit
that referenced
this pull request
Apr 21, 2015
Add new profling options for Facebook/hhvm
|
Going to revisit --i-am-not-benchmarking, this breaks its' purpose. |
fredemmott
added a commit
that referenced
this pull request
Apr 22, 2015
- require --i-am-not-benchmarking as they pretty much all have a potential perf impact - removing the alignment as it's inconsistent with the rest, and can't be maintained in the future while preserving git blame.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary: This is commit adds a hand full of convenience options for:
The following changes were also made:
The following is still broken on devservers:
The following features would be useful in a followup PR:
The additional warmup time was added after inspecting the tc-filling behavior and discovering that tc size was still dramatically increasing after 50 warmup requests. The additional serial warmup requests ensure near optimal tc layout, while the extra complete run of parallel requests is for a handful of frameworks that continue to substantially JIT even after a high volume of single threaded requests (mostly in conjunction with broken pseudomain JITing). While inconvenient the added warmup should ensure better consistency (this was certainly observed in testing). It's also particularly important in repo-auth mode where hot functions are JITed into Ahot after appropriate profiling which may further delay warmup.
Test Plan: Tested all new options, ran hh_client