perf: increment/decrement len without reading on heap flag

[fereidani]

It's pretty simple and it just bypasses reading the on heap flag by adding 0b10 when type is not ZST.

But I think there is another problem here. set_len is a unsafe operation, in this codebase we can set_len from safe code.
We must follow this by a refactor/fix and making it impossible to set_len from safe API to avoid future mistakes.
Maybe a UnsafeCell<usize> solution?

[alejandro-vaz]

yeah this is what #451 is trying to do more or less

would you want to implement it in full??

I do like your increment thing, maybe you can generalize it to AddAssign for TaggedLen / SubAssign

when a type is not a ZST, any increment of length by value is just value * 2 or value << 1 and then addition

it would also be helpful (a few notes I took):

• adding the ability to relocate (change the LSB without having to create a TaggedLen) fn relocate<const ONHEAP: bool>(&mut self)
• not having the TaggedLen API use an on_heap: bool argument but a const generic (it fully simplifies at compile time). it can be done by adding also relocate

[alejandro-vaz]

set_len is something that we have to try to remove, see #406 and the weird stuff we had to do

[fereidani]

yeah this is what #451 is trying to do more or less
would you want to implement it in full??

Yeah sure, I have 3 or 4 optimization related PRs for smallvec that I'm working on, I can do it after those, But don't block it because of me, if someone is interested let them do it.

I do like your increment thing, maybe you can generalize it to AddAssign for TaggedLen / SubAssign

I prefer const fn, I saw some weird stuff with generic code optimizations that for these low level stuff I prefer to keep it as simple as possible.

when a type is not a ZST, any increment of length by value is just value * 2 or value << 1 and then addition

What about putting the flag in MSB instead and always doing +1 and -1? we can use CPU overflow flags and implement it better. I need to review the codebase more carefully, I'm not sure that integer overflows are handled correctly.

[alejandro-vaz]

I did make a try at storing the flag on the MSB, it's basically the same, except you have to make a mask that only has the MSB on. I do agree that it seems that for repeated length changes it should be faster

I'm pretty sure as well that not all integer overflows are handled correctly. literally our current validation is debug_assert! doing all the work and praying that nothing in production hits it in a weird pattern

I don't think fuzzing tests that either

[fereidani]

Sorry for the force-push, I missed one decrement case.

Would you like to merge this one or you prefer the full rewrite?

[alejandro-vaz]

I prefer it now

[alejandro-vaz]

the full rewrite is probably going to take some time, so it's not a good idea to rush it to alpha.13

alpha.14 is for September 10

[alejandro-vaz]

whatever borsh serialization/deserialization does, miri spends a bunch of time on it

It needs like 10 minutes