| ▲ | Karliss 3 days ago | |
Except your giant comment doesn't actually explain why it used uint64. Only place mentioning uint64 is integer promotion which only happens because you used 64bit integer, thus no explanation of why. Was it done because shifting by amount equal or greater to integer width is undefined behavior? That would still not require storing result in 64bit mask, just shifting (~0ULL) would be enough. That would be a lot more valuable to explain than how bitwise AND works. The first one also seems slightly sketchy but without knowing rest of details it's hard to be sure. IPV6 address is 128bits, that's 2 registers worth integers. Calculating base address would take 2 bitwise instruction. Cost of copying them in most cases would be negligible compared to doing the lookup in whatever containers you are searching resulting address. If you are storing it as dynamically allocated byte arrays (which would make copying non trivial) and processing it in such a hot loop where it matters, then seems like you have much bigger problems. For my taste it would be sufficient to say "Iterate in reverse order from most specific address to least specific. That way address can be a calculated in place by incrementally clearing lowest bits." Having 2 paragraphs of text which repeat the same idea in different words is more distracting than it helps. | ||
| ▲ | commandersaki 3 days ago | parent [-] | |
Sorry, I didn't explain uint64 was used. I wrote this many years ago so my memory was foggy, but I went through a few iterations using uint32 only to use branches for the masks. This was the only branchless way I could come up with at the time after a few attempts. I think the example was more to demonstrate that the algorithm was correct and I wasn't going to unit test it at that scope. As for the 128-bit addresses, we used boost ip::address_v6 to_bytes() as it appears there was no masking option. For my taste it would be sufficient to say "Iterate in reverse order from most specific address to least specific. That way address can be a calculated in place by incrementally clearing lowest bits." Having 2 paragraphs of text which repeat the same idea in different words is more distracting than it helps. Ah apologies, too late now, should've mentioned it in the PR. But I expected it would ruffle some feathers, I don't care for conventions or other people's quibbles. As long as it improves understanding for the reader, regardless of how "redundant", then mission accomplished. | ||