xiangfu0 commented on code in PR #20249:
URL: https://github.com/apache/pinot/pull/20249#discussion_r4559079079
##########
pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/memory/PrimitiveArrays.java:
##########
@@ -21,24 +21,26 @@
import java.nio.ByteBuffer;
-/// Bulk reads of primitive arrays from a [ByteBuffer] through its typed
views, and the threshold below which one
-/// access per value beats a view. The class is stateless and thread-safe.
+/// Bulk reads of primitive arrays from a [ByteBuffer] through its typed
views. The class is stateless and thread-safe.
+///
+/// On a little-endian CPU, a copy through a big-endian view
(`asIntBuffer().get(int[])` and the like) swaps the bytes
+/// in a native call with a fixed cost of 20 to 40 ns, so a short read gets
one value at a time instead. With JMH on
+/// JDK 25, reading from heap and direct buffers held in a field (as a
deserializer gets them), the view was 14 to 24%
+/// faster than one value at a time at 128 longs or doubles on Intel Xeon
6973P-C, AMD EPYC 7763 and Neoverse N2
+/// machines, and 14 to 35% slower on an Apple M3 Max, where it caught up at
160 to 192. For ints and floats, one value
+/// at a time took several times as long in some JVM launches as in others on
those three machines, so no threshold
+/// suits every launch; on the M3 the view caught up at 96 to 128.
public final class PrimitiveArrays {
private PrimitiveArrays() {
}
- // TODO: Retune this threshold. JMH runs on aarch64 (Apple M3 Max, JDK 25)
put the crossover at about 110-200 values
- // for reads, 300-500 for int writes and beyond 4096 for double writes, so
a single threshold may not fit them all.
- /// Below this number of values, reads and writes through a big-endian typed
view (`asIntBuffer().get(int[])` and
- /// the like) are slower than one access per value. On a little-endian CPU
the bulk copy swaps the bytes in a native
- /// call with a fixed cost of about 10-40 ns, while a single-value access
compiles to an inlined byte swap. Measured
- /// with BenchmarkPrimitiveListDeserialize and
BenchmarkPagedPinotOutputStream on aarch64 and x86_64.
- public static final int MIN_BULK_VALUES = 64;
+ /// The `read*` methods read at least this many values through a typed view,
and fewer one at a time.
+ public static final int MIN_BULK_READ_VALUES = 128;
Review Comment:
On x86, 128 makes reads of 96 to 127 values slower than base. I measured
base vs head through `read*` on JDK 25.0.5 and a Xeon 6975P-C, with stable
forks unless noted:
- 127 longs: 57 → 66 ns (+12 to 16%).
- 127 doubles: +14 to 17%.
- 127 ints, heap: 53 → 70 ns (+31%).
- 127 floats, direct: 51 → 75 ns (+47%, in all 3 forks).
- At 96 values: +7 to 15%.
At 64 values, reads are 23% faster. Your table puts the crossover for long
and double reads at 64 to 96 on all three server CPUs; only the M3 favors 128.
If servers are the target, 96 may fit better. This is non-blocking.
##########
pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/memory/PrimitiveArrays.java:
##########
@@ -21,24 +21,26 @@
import java.nio.ByteBuffer;
-/// Bulk reads of primitive arrays from a [ByteBuffer] through its typed
views, and the threshold below which one
-/// access per value beats a view. The class is stateless and thread-safe.
+/// Bulk reads of primitive arrays from a [ByteBuffer] through its typed
views. The class is stateless and thread-safe.
+///
+/// On a little-endian CPU, a copy through a big-endian view
(`asIntBuffer().get(int[])` and the like) swaps the bytes
+/// in a native call with a fixed cost of 20 to 40 ns, so a short read gets
one value at a time instead. With JMH on
+/// JDK 25, reading from heap and direct buffers held in a field (as a
deserializer gets them), the view was 14 to 24%
+/// faster than one value at a time at 128 longs or doubles on Intel Xeon
6973P-C, AMD EPYC 7763 and Neoverse N2
+/// machines, and 14 to 35% slower on an Apple M3 Max, where it caught up at
160 to 192. For ints and floats, one value
Review Comment:
Nit: the PR table has 128 long/double reads on the M3 at 1.14 to 1.20 (14 to
20% slower), but this comment says up to 35%. Likewise,
`PagedPinotOutputStream`'s `MIN_BULK_WRITE_VALUES` doc says ints are 8 to 14%
slower and floats 2 to 5% faster on the M3, while the table has 1.08 to 1.11
and 0.95 to 0.96. Worth reconciling so the comments match the data.
--
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]
