[PATCH 1 of 6 V2] obsstore: add a 'cachekey' method

[Martin von Zweigbergk]

On Tue, Jun 6, 2017 at 10:39 AM, Martin von Zweigbergk
<martinvonz at google.com> wrote:
> On Tue, Jun 6, 2017 at 10:27 AM, Pierre-Yves David
> <pierre-yves.david at ens-lyon.org> wrote:
>>
>>
>> On 06/06/2017 06:01 PM, Martin von Zweigbergk wrote:
>>>
>>> Here are some timings from a hopefully unbiased third party. These are
>>> best-of-20 times, in seconds. No extensions enabled. I ran
>>> "debugupdatecaches" before each series of commands to populate the
>>> caches (I think both obscache and the radixlink cache are hooked into
>>> that command).
>>
>>
>> Can you provides timing for the combination of both. The two series are not
>> meant to compete but to collaborate. I've a merge available at:
>>
>>   hg pull -r cf7fd13ead00
>> https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
>
> Sure:
>
>                          |  At @   | obscache | radixlink | combined |
> id -r @                  |  0.70   |   0.38   |    0.42   |   0.42   |
> log -r @                 |  1.60   |   1.08   |    0.59   |   0.81   |
> export @                 |  0.71   |   0.33   |    0.59   |   0.40   |
> log -G -r 'draft()'      |  1.19   |   1.22   |    0.74   |   0.91   |
> log -G -r 'draft()' \    |  1.26   |   1.27   |    0.97   |   0.97   |
>   -T '{node} {troubles}' |         |          |           |          |

Pierre-Yves pointed out that I should run with "--config
experimental.evolution=all" to avoid loading the obsstore just to
print a warning about it. Here are the updated figures:

                         |  At @   | obscache | radixlink | combined |
log -r @                 |  1.14   |   1.05   |   0.33    |   0.63   |
export @                 |  0.73   |   0.11   |   0.33    |   0.12   |
log -G -r 'draft()'      |  1.58   |   1.22   |   0.76    |   0.50   |
log -G -r 'draft()' \    |  1.27   |   1.16   |   0.46    |   0.70   |
  -T '{node} {troubles}' |         |          |           |          |
id -r @                  |  0.81   |   0.11   |   0.67    |   0.11   |
log -G -r @ \            |  0.70   |   0.10   |   0.29    |   0.10   |
  -T '{node} {desc}'     |         |          |           |          |


I have no idea why "log -r @" got slower without radixlink than it was
without "--config experimental.evolution=all".

>
>
>>
>> Also, Can you give use some details on the repo caracteristic? I'm getting
>> significantly slower runtime for all of these commands in my repository.
>
>
> 33038 visible revisions
> 20872 hidden revisions
> 53910 total revisions
>
> first hidden revision: 22281
>
> 146322k obsmarkers
>
>
>>
>>                          |  At @   | obscache | radixlink |
>>>
>>> id -r @                  |  0.70   |   0.38   |    0.42   |
>>> log -r @                 |  1.60   |   1.08   |    0.59   |
>>> export @                 |  0.71   |   0.33   |    0.59   |
>>> log -G -r 'draft()'      |  1.19   |   1.22   |    0.74   |
>>> log -G -r 'draft()' \    |  1.26   |   1.27   |    0.97   |
>>>   -T '{node} {troubles}' |         |          |           |
>>
>>
>>
>>> We write Mercurial for users, so I've included commands similar to
>>> what I use somewhat often (plus "hg id" because Pierre-Yves and Jun
>>> like it).
>>>
>>> So radixlink seems to provide a nice speedup across the board. So I
>>> vote for getting that series in good enough shape to include it and
>>> then we can come *consider* adding obscache on top after. I know
>>> Pierre-Yves wanted to get the cachekey and dualsourcecache pieces in
>>> at least to simplify further work in the evolve extension, but I'm
>>> very reluctant to add otherwise unused code (more than just a few
>>> lines) to core for that reason. What do other reviewers think?
>>>
>>>
>>> On Sun, Jun 4, 2017 at 10:05 PM, Jun Wu <quark at fb.com> wrote:
>>>>
>>>> Excerpts from Pierre-Yves David's message of 2017-06-04 01:24:59 +0200:
>>>>>
>>>>> There is likely something wrong with your timing is you get a 10ms speed
>>>>> up… since the obscache computation is about 1ms. How are you running
>>>>> your testing? Do you have your code available somewhere?
>>>>>
>>>>> (note: you probably have an outdated version on obscache too, I shaved
>>>>> it a bit in the past week (1.58ms → 0.95ms))
>>>>
>>>>
>>>> Since I have sent the complete version of radixlink series, I'd like to
>>>> comment on (re-clarify) some key points:
>>>>
>>>>   1. obscache puts Facebook at risk of spending about 10 seconds for a
>>>>      simple command like "hg id", even if the obsstore is tiny.
>>>>
>>>>      That could happen when obscache needs to be rebuilt. Like after a
>>>> fresh
>>>>      clone, after a real strip (histedit --abort does a real strip).
>>>>
>>>>      radixlink does not have such issue - without a prebuilt index, it's
>>>>      even a bit faster than what we have today.
>>>>
>>>>   2. radixlink speeds up all 4 revsets (obsolete, unstable, bumped,
>>>>      divergent), while obscache only works for obsolete.
>>>>
>>>>      I admit it could be 20ms slower for the "obsolete" set. But if I
>>>> really
>>>>      want to work in this area, I believe I can catch up by implementing
>>>>      efficient laziness (that also applies to other 3 sets).
>>>>
>>>>   3. Looking at the future where hash-preserving obsstore is a thing,
>>>>      radixlink is compatible with it out-of-box. obscache has to be
>>>> reworked
>>>>      to support it.
>>>>
>>>> I think there is little reason that we want the obscache series. I hope
>>>> someone from the committee could see through and make the right decision.
>>>>
>>>>> [...]
>>>>
>>>> _______________________________________________
>>>> Mercurial-devel mailing list
>>>> Mercurial-devel at mercurial-scm.org
>>>> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>>
>>
>> --
>> Pierre-Yves David