Reorganize decode_varint to remove an unsafe block

[alex]

The generated assembly for this is identical to the previous.

[alex]

Failing tests don't seem related?

[danburkert]

@alex how did you validate the assembly? Afaict from cargo asm output, the decode_varint method is always inlined. This patch appears to significantly regress the varint decoding microbenchmarks, however I don't currently have access to my usual benchmarking machine.

[alex]

https://rust.godbolt.org/ - IIRC I copied in the minimal amount of source for the buf trait and implementation

[Shnatsel]

It seems this was approved but never merged. What's blocking this?

[danburkert]

@Shnatsel this patch unfortunately regresses performance measurably in the in-tree benchmark suite (both micro varint decoding, as well as macro message decoding). As such I'm not in favor of landing it, since I think the unsafe usage is low risk. I'm going to keep it open since I think the problem may be solvable, but at the moment this is not on track to get merged.

[danburkert]

Note to self: I haven't re-tested this since the bytes 0.6 update, so it's possible that the perf regression is no longer present.

[LucioFranco]

Re-ran this bench against master and still seeing a regression in decoding varints

varint/medium/decode    time:   [522.27 ns 522.97 ns 523.68 ns]                                  
                        change: [+14.713% +14.934% +15.153%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 6 outliers among 100 measurements (6.00%)
  3 (3.00%) low mild
  3 (3.00%) high mild

varint/mixed/decode     time:   [553.71 ns 553.78 ns 553.85 ns]                                 
                        change: [+12.023% +12.083% +12.144%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 3 outliers among 100 measurements (3.00%)
  1 (1.00%) low mild
  2 (2.00%) high severe

varint/large/decode     time:   [725.44 ns 727.34 ns 730.29 ns]                                 
                        change: [+2.0304% +2.4131% +2.9371%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 3 outliers among 100 measurements (3.00%)
  2 (2.00%) high mild
  1 (1.00%) high severe

So I am going to go ahead and close the PR inline with what Dan said above. Thanks for the contribution!

[DemiMarie]

This looks like a compiler problem to me.