hhr293 commented on code in PR #12299:
URL: https://github.com/apache/gluten/pull/12299#discussion_r3412278858
##########
cpp/core/utils/tac/ffor.hpp:
##########
@@ -59,6 +60,13 @@
namespace gluten {
namespace ffor {
+// Byte order (applies to the 128-bit codec below): this codec round-trips
+// data as native uint64 reads/writes against the lo (offset 0) and hi
+// (offset 8) halves of each 128-bit value (DECIMAL128's in-memory layout in
+// Velox). Producer and consumer share byte order by virtue of running in
+// the same Spark cluster -- LZ4 and any other shuffle codec carry the same
+// implicit assumption -- so no explicit endian guard is needed here.
Review Comment:
The lo/hi half ordering (lo at offset 0, hi at offset 8) is actually
dictated by the __int128_t ABI layout on all supported Velox platforms (x86-64,
ARM64). It is a structural property of the type itself rather than a runtime
byte-order assumption.
Furthermore, Velox inherently assumes a little-endian environment across the
board (e.g., in Arrow IPC, Parquet, and general memory layouts) and does not
support big-endian targets. Introducing a compile-time endian guard here would
be inconsistent with the rest of the codebase and would guard against a
configuration that cannot exist in practice.
The current setup relies on the fact that the producer and consumer share
the same byte order within the cluster—an assumption already safely relied upon
by LZ4, ZSTD, and the rest of our shuffle stack.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
