[Commented On] D9098: hg-core: avoid memory allocation (D8958#inline-14990 followup)

[martinvonz (Martin von Zweigbergk)]

martinvonz added a comment.


  In D9098#136670 <https://phab.mercurial-scm.org/D9098#136670>, @Alphare wrote:
  
  > In D9098#136662 <https://phab.mercurial-scm.org/D9098#136662>, @martinvonz wrote:
  >
  >> In D9098#136605 <https://phab.mercurial-scm.org/D9098#136605>, @Alphare wrote:
  >>
  >>> I personally find this harder to read
  >>
  >> I find it easier to read :)
  >
  > heh
  >
  >>> , and the use of `usize` before bit shifting making this technically platform-dependent makes me a bit uneasy.
  >>
  >> It probably doesn't matter if we're truncating (on small platforms) the result to `usize` anyway? I suppose we can use `usize::try_from()` to check (whether or not we make the change in this patch).
  >>
  >>> I can't easily compare the compiler output since godbolt does not support crates and I'd have to find out how to do it by hand, but maybe you or @martinvonz did?
  >>
  >> I hadn't, but I just tried it using the `criterion` benchmarking crate. This is the code I used:
  >>
  >>   let input: Vec<u8> = (0..60000).map(|_| { rand::random::<u8>() }).collect();
  >>   let routine = || {
  >>       for i in 0..input.len()/6 {
  >>           let slice = &input[6*i..6*i+6];
  >>           let val = ((BigEndian::read_u16(&slice[0..2]) as usize) << 32)
  >>               + (BigEndian::read_u32(&slice[2..6]) as usize);
  >>       }
  >>   };
  >>
  >> Criterion says that it takes around 5.43 us both before and after this patch. So I guess the compiler is smart enough to eliminate the allocation, or maybe I messed something up.
  >
  > Seems fine indeed.
  
  To check that I hadn't messed something up, I ran the benchmark on a debug build and there this patch speeds it up from 1.51ms to 1.38ms (which is still ~240x as slow as the release build! :) ).

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D9098/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D9098

To: acezar, #hg-reviewers, Alphare
Cc: Alphare, martinvonz, mercurial-patches
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mercurial-scm.org/pipermail/mercurial-patches/attachments/20200928/33a3600e/attachment-0002.html>