c icon indicating copy to clipboard operation
c copied to clipboard

Cheaper and faster prehash generation mechanism

Open szaydel opened this issue 5 years ago • 5 comments

I am not sure if you have much interest in this, but I found that this method of creating id is no less reliable and faster as well as cheaper.

I must admit I have not done exhaustive testing, but if you find this interesting I am happy to do a good bit more validation. In my limited testing it has been reliable.

Ran a few comparisons just so there's something I can show. The only difference is this changeset.

These are my two test scripts...

#!/bin/sh
echo "*** Run 1000 iterations new method ***"
for i in `seq 0 999`; do
    /home/ubuntu/bin/c.new "redis-h1get.c -ggdb -Wall -I/usr/include/hiredis `pkg-config --libs --cflags hiredis`" h1 tags > /dev/null
done
echo "*** Done running new method ***"
#!/bin/sh
echo "*** Run 1000 iterations original method ***"
for i in `seq 0 999`; do
    /home/ubuntu/bin/c "redis-h1get.c -ggdb -Wall -I/usr/include/hiredis `pkg-config --libs --cflags hiredis`" h1 tags > /dev/null
done
echo "*** Done original method ***"
*** Run 1000 iterations new method ***
*** Done running new method ***
3.56user 6.32system 0:38.73elapsed 25%CPU (0avgtext+0avgdata 3280maxresident)k
0inputs+16000outputs (0major+2885064minor)pagefaults 0swaps

*** Run 1000 iterations new method ***
*** Done running new method ***
3.48user 6.51system 0:38.55elapsed 25%CPU (0avgtext+0avgdata 3304maxresident)k
0inputs+16000outputs (0major+2890112minor)pagefaults 0swaps


*** Run 1000 iterations original method ***
*** Done original method ***
17.10user 19.30system 1:02.29elapsed 58%CPU (0avgtext+0avgdata 8440maxresident)k
0inputs+112000outputs (0major+3196581minor)pagefaults 0swaps

*** Run 1000 iterations original method ***
*** Done original method ***
16.76user 20.06system 1:01.91elapsed 59%CPU (0avgtext+0avgdata 8420maxresident)k
0inputs+112000outputs (0major+3196095minor)pagefaults 0swaps

Thanks!

szaydel avatar Aug 03 '18 22:08 szaydel

Nice! Yeah, that method looks a lot faster.

It seems that all tests are passing, so I don't see a reason not to accept this. I'll take a deeper look at the code itself tomorrow.

Nice work!

ryanmjacobs avatar Aug 04 '18 01:08 ryanmjacobs

Sounds good. I am sure one can find that this method could be improved. Let me know if you find any issues you feel are non-starters, and I can see about addressing them as time allows.

And, thanks again!

szaydel avatar Aug 04 '18 01:08 szaydel

@ryanmjacobs, I was wondering if we should just close this, or if you have interest still, I am happy to fix it, to make sure it could be merged in without any conflicts.

szaydel avatar Jul 05 '19 14:07 szaydel

Hi @szaydel, I'm okay with merging this as long as I'm 100% certain that it doesn't break anything.

In terms of efficiency, this bumps it down from ~17 ms to ~4 ms. Generally, if a UI element performs in less than 100 ms, nobody will notice anyways. I'm okay with sacrificing 15 ms for correctness.

That being said...

My primary concern is that this doesn't run the file through cpp (the C Preprocessor). For example, when you compile this code...

int main(void) {
#ifdef __unix__
  return 1;
#endif  

  return 0;
}

c should re-compile the code if __unix__ happens to be redefined. But without running through the C preprocessor, the hash will never change (because the source code technically never changed).

ryanmjacobs avatar Jul 05 '19 15:07 ryanmjacobs

Fair point about the preprocessor. I guess I am not 100% certain that this does not break anything, at least in my testing it did not. Why don't we close this out for now, and I will play more with this to make sure I am very confident first, and then we can open this conversation up again.

szaydel avatar Jul 06 '19 01:07 szaydel