Speed up Send/Receive operations significantly - #258
Conversation
3cd1ff2 to
bc748eb
Compare
The Send/Receive code does a lot of `hg log -rREV --template "{node}"`
calls in order to learn the long hash of a revision number. This happens
very often during a Send/Receive process: the Chorus logs are full of
`hg log -r0` operations. Thing is, every single call to `hg log` incurs
a cost as a Python process has to be spun up every time this happens.
And in any given repository, a specific revision number wil always refer
to the same revision with the same long hash, so each long hash only has
to be calculated *once* for any given revision number. So we'll cache
the output of `log -rREV` operations and return the cached output if the
same operation runs a second time. (Other operations like `hg parents`
aren't safe to cache, as their result will change during a Send/Receive
operation as new commits are added to the repo. Only `log -rREV` calls
are 100% safe to cache).
bc748eb to
5160d4e
Compare
Make ExecuteErrorsOk an instance method since there's no reason it needs to be static (no code that uses the Chorus library can be calling it since it's private, so this won't affect any other code). This also allows simplifying its type signature.
6454df5 to
ef08bf8
Compare
Elsewhere the `command` parameter is used without checking for nullity, so LGTM.com considers my null check to add a new possible problem. Since this is an internal method and we're pretty much never going to pass a null command string in here, we can remove that null check and be okay.
|
I wasn't seeing any occurrences of the "Using cached result" message in the Chorus logs while TeamCity ran the unit tests, and I just found out from looking at the logs (and then the source) that On the plus side, this means I can compare the time it took TeamCity to run the unit tests before and after commit 05fa900 and I'll have a poor man's benchmark. I still want to run a proper benchmark, but that'll help me get initial speedup numbers. |
|
Preliminary results: the TeamCity build including unit tests took 1374 seconds before commit 05fa900 went in (meaning it wasn't actually running the optimizations) and the build after commit 05fa900 went in took 858 seconds. That's 62.5% of the previous value, which means that 1/3 of the build's time was being spent running Unfortunately, now four unit tests are failing because they relied on First thought: add some logic to the caching to examine the output and, if (and only if) the output is an all-zeroes value, do not cache that value as it's going to change in the near future. That should, I think, take care of all four of the failing unit tests. But I'm running out of time to test that today. |
An all-zero result from hg is highly likely to be the repo identifier of an empty repository, which is going to change in the near future. So we'll skip caching anything that starts with twelve zeroes in a row. This will occasionally have a false positive, but that false positive won't have any negative impact (a minor slowdown).
|
Bingo. That worked. One more commit and I'll feel ready to merge this. I wrote the check to use only a dozen zeroes, but I think I should compare against the full all-0 SHA value, which is encoded somewhere in the code, so I can use a reference instead of a magic string. Once that commit is merged and tests passing, I'll merge this. |
This avoids using magic strings.
7eb3485 to
76fb646
Compare
Negative revision numbers mean "counting backwards from tip", i.e.
running `hg log -r-1 --template "{node}" gets the long hash of the tip
revision. We therefore should not cache negative revision numbers as
those are subject to change.
2 participants
The Send/Receive code does a lot of
hg log -rREV --template "{node}"calls in order to learn the long hash of a revision number. This happens very often during a Send/Receive process: the Chorus logs are full ofhg log -r0operations. Thing is, every single call tohg logincurs a cost as a Python process has to be spun up every time this happens. And in any given repository, a specific revision number wil always refer to the same revision with the same long hash, so each long hash only has to be calculated once for any given revision number. So we'll cache the output oflog -rREVoperations and return the cached output if the same operation runs a second time. (Other operations likehg parentsaren't safe to cache, as their result will change during a Send/Receive operation as new commits are added to the repo. Onlylog -rREVcalls are 100% safe to cache).This is PR #257 applied to
master(#257 is applied to thelfmergebranch) as suggested in #257 (review)This change is