On Thu, 20 Mar 2025 13:55:11 GMT, Michael McMahon <micha...@openjdk.org> wrote:
>> Volkan Yazici has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Apply review feedback on naming in `Masker` >> >> `reset` -> `setMask` >> `initGallopingMask` -> `initVectorMask` >> `applyGallopingMask` -> `applyVectorMask` > >> Hi, original filer here. I don't know how you handle such things here, but >> 24 files changed seems somewhat excessive to me for this bug. >> >> I do not know how effective the long-optimization realy is because/when data >> is not 8-byte-aligned. But if one wants to implement it, be sure to apply >> _native_ endianess for the mask-long, the input (eg. via a slice) and the >> buffer. If one uses the java default big endian (aaargh...), and "happens" >> to run on a little endian machine, the "optimization" would have to do a >> byte swap to native endian on read, do the masking and byteswap again on the >> way out. (same is true when choosing little endian and running on big endian >> hardware, although i would like the chances there much better ;-)) > > I don't think alignment is an issue here at all. `ByteBuffer.getLong()` does > not have any alignment constraints on the underlying memory. There's no way > to know if it is 8-byte aligned afaict. The first 8 bytes (and every > subsequent 8 bytes) can be read as a long (subject to this byte ordering > issue). The final chunk of up to 7 bytes is then handled byte at a time. > > To add to that; it's probably still true that the optimization would not be > very effective if the buffers are not 8 (or at least 4) byte aligned, but > there's no way to guarantee it. > > I'm not sure what (if any) difference the platform's native byte order has > either for that matter. @Michael-Mc-Mahon, after having a conversation with @dfuch, I've expanded the tests to cover cases where negative `byte` values are present both in the mask and the in the input buffer to be masked. I'd appreciate your final review, and if you don't have any objections, approval, please. ------------- PR Comment: https://git.openjdk.org/jdk/pull/24033#issuecomment-2778488190
