Re: xdelta speedups (was Re: [Monotone-devel] updates to net.venge.monotone.experiment.performance)

[Nathaniel Smith]

On Thu, Aug 10, 2006 at 01:06:03AM -0700, Eric Anderson wrote:
> Repaired in 40accd16c45a4195e207b91c1d9dec6d6a8170cb; I think I got
> all of them.  Is there a CodingStyle file somewhere that explains all
> the rules that should be applied so I can just verify them when I
> split out to a patch against mainline?

HACKING is the closest thing; it basically just says "use GNU style,
unless the file already uses something else", and also mentions the
rules about writing things like "int const * foo".  I guess it could
potentially mention "don't use C-style casts", though that's just good
C++ sense :-).

> On Wed, Aug 09, 2006 at 10:42:37PM -0700, Eric Anderson wrote:
>  > I am unsure about this, simply because I don't know what widen<> is
>  > for in the first place.  Graydon, can you shed any light?
> 
> My best guess was that widen was there to make sure that when widening
> a signed to an unsigned the right thing happened, but I wasn't sure it
> seemed unnecessary for an unsigned to unsigned widen.

Ah-hah, that seems plausible.  It would certainly be possible to teach
widen<> to compile out to a simple cast in case both sides are
unsigned (it already compiles out if the target type is signed, which
matches your thought).  That would probably be a bit cleaner than
manually verifying when it is safe and doing that replacement
manually, as this patch ends up doing ATM.

>  > +  // selfcheck by making short-distance test true || badvance <= blocksz
>  > +  //              u32 save_sum = rolling.sum();
>  > +  rolling.replace_with(reinterpret_cast<u8 const *>(b.data() + new_lo), 
> new_hi-new_lo);
>  > +  //              I(rolling.sum() == save_sum);
>  > 
>  > Comments here make me go "huh?".
> 
> I expanded the comment a lot, it's just a way to verify that the
> skip-forward checksum matches the rolling checksum.  It passed on a
> complete pull of the monotone database, but I thought others might
> want to verify.

If you're worried, maybe there should be a unit test, running both on
some random data?

-- Nathaniel

-- 
Damn the Solar System.  Bad light; planets too distant; pestered with
comets; feeble contrivance; could make a better one myself.
  -- Lord Jeffrey