LiangliangSui commented on code in PR #1434:
URL: https://github.com/apache/incubator-fury/pull/1434#discussion_r1554626407
##########
java/fury-core/src/main/java/org/apache/fury/memory/MemoryBuffer.java:
##########
@@ -1233,49 +1233,9 @@ public int readVarInt() {
* to avoid using two memory operations.
*/
public int unsafeWritePositiveVarInt(int v) {
- // The encoding algorithm are based on kryo UnsafeMemoryOutput.writeVarInt
- // varint are written using little endian byte order.
- // This version should have better performance since it remove an index
update.
- long value = v;
- final int writerIndex = this.writerIndex;
- long varInt = (value & 0x7F);
- value >>>= 7;
- if (value == 0) {
- UNSAFE.putByte(heapMemory, address + writerIndex, (byte) varInt);
- this.writerIndex = writerIndex + 1;
- return 1;
- }
- // bit 8 `set` indicates have next data bytes.
- varInt |= 0x80;
- varInt |= ((value & 0x7F) << 8);
- value >>>= 7;
- if (value == 0) {
- unsafePutInt(writerIndex, (int) varInt);
- this.writerIndex = writerIndex + 2;
- return 2;
- }
- varInt |= (0x80 << 8);
- varInt |= ((value & 0x7F) << 16);
- value >>>= 7;
- if (value == 0) {
- unsafePutInt(writerIndex, (int) varInt);
- this.writerIndex = writerIndex + 3;
- return 3;
- }
- varInt |= (0x80 << 16);
- varInt |= ((value & 0x7F) << 24);
- value >>>= 7;
- if (value == 0) {
- unsafePutInt(writerIndex, (int) varInt);
- this.writerIndex = writerIndex + 4;
- return 4;
- }
- varInt |= (0x80L << 24);
- varInt |= ((value & 0x7F) << 32);
- varInt &= 0xFFFFFFFFFL;
- unsafePutLong(writerIndex, varInt);
- this.writerIndex = writerIndex + 5;
- return 5;
+ int varintBytes = unsafePutPositiveVarInt(writerIndex, v);
Review Comment:
After passing the benchmark, it can still be inlined and there is no
performance regression.
The difference between the two is 155266, which should be within the error.
In addition, I saw in the log that `unsafePutPositiveVarInt` can be inlined
in the benchmark process by using `-XX:+PrintCompilation
-XX:+UnlockDiagnosticVMOptions -XX:+PrintInlinin`
```
@ 152 java.lang.Integer::intValue (5 bytes) accessor
@ 155 org.apache.fury.memory.MemoryBuffer::writeVarInt (17 bytes) inline
(hot)
@ 8 org.apache.fury.memory.MemoryBuffer::ensure (37 bytes) inline (hot)
@ 13 org.apache.fury.memory.MemoryBuffer::unsafeWriteVarInt (15 bytes)
inline (hot)
@ 11 org.apache.fury.memory.MemoryBuffer::unsafeWritePositiveVarInt
(22 bytes) inline (hot)
@ 6 org.apache.fury.memory.MemoryBuffer::unsafePutPositiveVarInt
(208 bytes) inline (hot)
@ 39 sun.misc.Unsafe::putByte (11 bytes) force inline by
annotation
@ 7 jdk.internal.misc.Unsafe::putByte (0 bytes) (intrinsic)
@ 81 org.apache.fury.memory.MemoryBuffer::unsafePutInt (45 bytes)
inline (hot)
@ 23 sun.misc.Unsafe::putInt (11 bytes) force inline by
annotation
@ 7 jdk.internal.misc.Unsafe::putInt (0 bytes) (intrinsic)
```
--
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: commits-unsubscr...@fury.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@fury.apache.org
For additional commands, e-mail: commits-h...@fury.apache.org
