Code review comment for lp:~spiv/bzr/inventory-delta

John A Meinel wrote:
[...]
> So something isn't quite right with my timings as:
>
> wbzr init-repo --2a test-2a
> time wbzr push -d ../bzr/bzr.dev -r 2000 test-2a/bzr -DIDS:always
> 11m12.889s
>
> I wonder if you didn't make a mistake in your timing of IDS.

Yeah, that's likely, I think I probably forgot to init --2a on that run. (I
think I reran it with the correct setup then copy-&-pasted the number from the
wrong one). The other numbers should be fine.

My corrected time for that is 23m 1s! bzr.dev's IDS is much slower for me (29.5
minutes), so I certainly haven't regressed the performance. And the test suite
says I haven't regressed the correctness.

So my patch is looking better and better...

> In my timing of IDS versus InventoryDelta for bzrtools, it was more:
>
> 15.8s time wbzr push -d bzrtools bzr://localhost/test-2a/bzrt
> 19.1s time wbzr push -d bzrtools test-2a/bzrt
>
> Which shows that IDS was actually *slower* than pushing using InventoryDelta
> over the local loopback.

Right, I'm seeing that too with bzr -r2000. (And glancing at the log, I think
it's still slower even if you don't count the time IDS spends autopacking.)

[...]
> I'll be running a couple more tests to see if the new refactoring of IDS that
> you've done has made anything slower, but at least at a first glance the only
> thing I could find that would be better with IDS is that it doesn't have a
> second pass over all inventories in order to generate the root texts keys. And
> that certainly wouldn't explain 9.5m => 1.0m.

I've taken another look at my refactoring of IDS, and I don't see any obvious
problems with my refactoring.

> I suggest you run your timing test again, and make sure you've set everything
> up correctly.
>
> I at least thought my laptop was faster than yours, though I'm on Windows and
> you may have upgraded your laptop since then.

I haven't upgraded my laptop for, well, years :)

> $ time wbzr push -d ../bzr/bzr.dev -r 2000 bzr://localhost/test-2a/bzr -DIDS:never
> real 4m32.578s
>
> This is 4m32s down from 11m12s for IDS (file:// to file://). Maybe something
> did get broken. I'll be running some more tests.

Where did you get to with your tests?

-Andrew.