You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by "ibessonov (via GitHub)" <gi...@apache.org> on 2023/06/23 15:05:29 UTC

[GitHub] [ignite-3] ibessonov opened a new pull request, #2249: IGNITE-19806 Optimize RAFT log encoder/decoder data format

ibessonov opened a new pull request, #2249:
URL: https://github.com/apache/ignite-3/pull/2249

   https://issues.apache.org/jira/browse/IGNITE-19828


-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [ignite-3] ibessonov commented on a diff in pull request #2249: IGNITE-19828 Optimize RAFT log encoder/decoder data format

Posted by "ibessonov (via GitHub)" <gi...@apache.org>.
ibessonov commented on code in PR #2249:
URL: https://github.com/apache/ignite-3/pull/2249#discussion_r1243598564


##########
modules/raft/src/main/java/org/apache/ignite/raft/jraft/entity/codec/v1/V1Decoder.java:
##########
@@ -51,98 +52,127 @@ public LogEntry decode(final byte[] content) {
         return log;
     }
 
+    // Refactored to look closer to Ignites code style.
     public void decode(final LogEntry log, final byte[] content) {
-        // 1-5 type
-        final int iType = Bits.getInt(content, 1);
-        log.setType(EnumOutter.EntryType.forNumber(iType));
-        // 5-13 index
-        // 13-21 term
-        final long index = Bits.getLong(content, 5);
-        final long term = Bits.getLong(content, 13);
+        var reader = new Reader(content);
+        reader.pos = LogEntryV1CodecFactory.PAYLOAD_OFFSET;
+
+        int typeNumber = (int)reader.readLong();
+        EnumOutter.EntryType type = Objects.requireNonNull(EnumOutter.EntryType.forNumber(typeNumber));
+        log.setType(type);
+
+        long index = reader.readLong();
+        long term = reader.readLong();
         log.setId(new LogId(index, term));
-        // 21-29 checksum
-        log.setChecksum(Bits.getLong(content, 21));
-        // 29-33 peer count
-        int peerCount = Bits.getInt(content, 29);
-        // peers
-        int pos = 33;
-        if (peerCount > 0) {
-            List<PeerId> peers = new ArrayList<>(peerCount);
-            while (peerCount-- > 0) {
-                final short len = Bits.getShort(content, pos);
-                final byte[] bs = new byte[len];
-                System.arraycopy(content, pos + 2, bs, 0, len);
-                // peer len (short in 2 bytes)
-                // peer str
-                pos += 2 + len;
-                final PeerId peer = new PeerId();
-                peer.parse(AsciiStringUtil.unsafeDecode(bs));
-                peers.add(peer);
+
+        long checksum = Bits.getLong(content, reader.pos);
+        log.setChecksum(checksum);
+
+        int pos = reader.pos + Long.BYTES;
+
+        // Peers and learners.
+        if (type != EnumOutter.EntryType.ENTRY_TYPE_DATA) {
+            reader.pos = pos;
+            int peerCount = (int)reader.readLong();
+            pos = reader.pos;
+            if (peerCount > 0) {
+                List<PeerId> peers = new ArrayList<>(peerCount);
+
+                pos = readNodesList(pos, content, peerCount, peers);
+
+                log.setPeers(peers);
             }
-            log.setPeers(peers);
-        }
-        // old peers
-        int oldPeerCount = Bits.getInt(content, pos);
-        pos += 4;
-        if (oldPeerCount > 0) {
-            List<PeerId> oldPeers = new ArrayList<>(oldPeerCount);
-            while (oldPeerCount-- > 0) {
-                final short len = Bits.getShort(content, pos);
-                final byte[] bs = new byte[len];
-                System.arraycopy(content, pos + 2, bs, 0, len);
-                // peer len (short in 2 bytes)
-                // peer str
-                pos += 2 + len;
-                final PeerId peer = new PeerId();
-                peer.parse(AsciiStringUtil.unsafeDecode(bs));
-                oldPeers.add(peer);
+
+            reader.pos = pos;
+            int oldPeerCount = (int)reader.readLong();
+            pos = reader.pos;
+            if (oldPeerCount > 0) {
+                List<PeerId> oldPeers = new ArrayList<>(oldPeerCount);
+
+                pos = readNodesList(pos, content, oldPeerCount, oldPeers);
+
+                log.setOldPeers(oldPeers);
             }
-            log.setOldPeers(oldPeers);
-        }
-        // learners
-        int learnersCount = Bits.getInt(content, pos);
-        pos += 4;
-        if (learnersCount > 0) {
-            List<PeerId> learners = new ArrayList<>(learnersCount);
-            while (learnersCount-- > 0) {
-                final short len = Bits.getShort(content, pos);
-                final byte[] bs = new byte[len];
-                System.arraycopy(content, pos + 2, bs, 0, len);
-                // peer len (short in 2 bytes)
-                // peer str
-                pos += 2 + len;
-                final PeerId peer = new PeerId();
-                peer.parse(AsciiStringUtil.unsafeDecode(bs));
-                learners.add(peer);
+
+            reader.pos = pos;
+            int learnersCount = (int)reader.readLong();
+            pos = reader.pos;
+            if (learnersCount > 0) {
+                List<PeerId> learners = new ArrayList<>(learnersCount);
+
+                pos = readNodesList(pos, content, learnersCount, learners);
+
+                log.setLearners(learners);
+            }
+
+            reader.pos = pos;
+            int oldLearnersCount = (int)reader.readLong();
+            pos = reader.pos;
+            if (oldLearnersCount > 0) {
+                List<PeerId> oldLearners = new ArrayList<>(oldLearnersCount);
+
+                pos = readNodesList(pos, content, oldLearnersCount, oldLearners);
+
+                log.setOldLearners(oldLearners);
             }
-            log.setLearners(learners);
         }
-        // old learners
-        int oldLearnersCount = Bits.getInt(content, pos);
-        pos += 4;
-        if (oldLearnersCount > 0) {
-            List<PeerId> oldLearners = new ArrayList<>(oldLearnersCount);
-            while (oldLearnersCount-- > 0) {
-                final short len = Bits.getShort(content, pos);
-                final byte[] bs = new byte[len];
-                System.arraycopy(content, pos + 2, bs, 0, len);
-                // peer len (short in 2 bytes)
-                // peer str
-                pos += 2 + len;
-                final PeerId peer = new PeerId();
-                peer.parse(AsciiStringUtil.unsafeDecode(bs));
-                oldLearners.add(peer);
+
+        // Data.
+        if (type != EnumOutter.EntryType.ENTRY_TYPE_CONFIGURATION) {
+            if (content.length > pos) {
+                int len = content.length - pos;
+
+                ByteBuffer data = ByteBuffer.wrap(content, pos, len).slice();
+
+                log.setData(data);
             }
-            log.setOldLearners(oldLearners);
         }
+    }
+
+    private static int readNodesList(int pos, byte[] content, int count, List<PeerId> nodes) {
+        while (count --> 0) {

Review Comment:
   Come on man!



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [ignite-3] SammyVimes commented on a diff in pull request #2249: IGNITE-19828 Optimize RAFT log encoder/decoder data format

Posted by "SammyVimes (via GitHub)" <gi...@apache.org>.
SammyVimes commented on code in PR #2249:
URL: https://github.com/apache/ignite-3/pull/2249#discussion_r1243605813


##########
modules/raft/src/main/java/org/apache/ignite/raft/jraft/entity/codec/v1/V1Encoder.java:
##########
@@ -46,130 +48,137 @@ public byte[] encode(final LogEntry log) {
         List<PeerId> oldLearners = log.getOldLearners();
         ByteBuffer data = log.getData();
 
-        // magic number 1 byte
-        int totalLen = 1;
-        final int iType = type.getNumber();
-        final long index = id.getIndex();
-        final long term = id.getTerm();
-        // type(4) + index(8) + term(8) + checksum(8)
-        totalLen += 4 + 8 + 8 + 8;
-        int peerCount = 0;
-        // peer count
-        totalLen += 4;
-        final List<String> peerStrs = new ArrayList<>();
-        if (peers != null) {
-            peerCount = peers.size();
-            for (final PeerId peer : peers) {
-                final String peerStr = peer.toString();
-                // peer len (short in 2 bytes)
-                // peer str
-                totalLen += 2 + peerStr.length();
-                peerStrs.add(peerStr);
-            }
-        }
-        int oldPeerCount = 0;
-        // old peer count
-        totalLen += 4;
-        final List<String> oldPeerStrs = new ArrayList<>();
-        if (oldPeers != null) {
-            oldPeerCount = oldPeers.size();
-            for (final PeerId peer : oldPeers) {
-                final String peerStr = peer.toString();
-                // peer len (short in 2 bytes)
-                // peer str
-                totalLen += 2 + peerStr.length();
-                oldPeerStrs.add(peerStr);
-            }
-        }
-        int learnerCount = 0;
-        // peer count
-        totalLen += 4;
-        final List<String> learnerStrs = new ArrayList<>();
-        if (learners != null) {
-            learnerCount = learners.size();
-            for (final PeerId learner : learners) {
-                final String learnerStr = learner.toString();
-                // learner len (short in 2 bytes)
-                // learner str
-                totalLen += 2 + learnerStr.length();
-                learnerStrs.add(learnerStr);
-            }
-        }
-        int oldLearnerCount = 0;
-        // old peer count
-        totalLen += 4;
-        final List<String> oldLearnerStrs = new ArrayList<>();
-        if (oldLearners != null) {
-            oldLearnerCount = oldLearners.size();
-            for (final PeerId oldLearner : oldLearners) {
-                final String learnerStr = oldLearner.toString();
-                // oldLearner len (short in 2 bytes)
-                // oldLearner str
-                totalLen += 2 + learnerStr.length();
-                oldLearnerStrs.add(learnerStr);
-            }
+        int totalLen = LogEntryV1CodecFactory.PAYLOAD_OFFSET;
+        int typeNumber = type.getNumber();
+        long index = id.getIndex();
+        long term = id.getTerm();
+
+        totalLen += sizeInBytes(typeNumber) + sizeInBytes(index) + sizeInBytes(term) + 8;
+
+        List<String> peerStrs = null;
+        List<String> oldPeerStrs = null;
+        List<String> learnerStrs = null;
+        List<String> oldLearnerStrs = null;
+
+        if (type != EntryType.ENTRY_TYPE_DATA) {
+            peerStrs = new ArrayList<>();
+            totalLen += nodesListSizeInBytes(peers, peerStrs);
+
+            oldPeerStrs = new ArrayList<>();
+            totalLen += nodesListSizeInBytes(oldPeers, oldPeerStrs);
+
+            learnerStrs = new ArrayList<>();
+            totalLen += nodesListSizeInBytes(learners, learnerStrs);
+
+            oldLearnerStrs = new ArrayList<>();
+            totalLen += nodesListSizeInBytes(oldLearners, oldLearnerStrs);
         }
 
-        final int bodyLen = data != null ? data.remaining() : 0;
-        totalLen += bodyLen;
+        if (type != EntryType.ENTRY_TYPE_CONFIGURATION) {
+            int bodyLen = data != null ? data.remaining() : 0;
+            totalLen += bodyLen;
+        }
 
-        final byte[] content = new byte[totalLen];
-        // {0} magic
+        byte[] content = new byte[totalLen];
         content[0] = LogEntryV1CodecFactory.MAGIC;
-        // 1-5 type
-        Bits.putInt(content, 1, iType);
-        // 5-13 index
-        Bits.putLong(content, 5, index);
-        // 13-21 term
-        Bits.putLong(content, 13, term);
-        // checksum
-        Bits.putLong(content, 21, log.getChecksum());
-
-        // peers
-        // 21-25 peer count
-        Bits.putInt(content, 29, peerCount);
-        int pos = 33;
-        for (final String peerStr : peerStrs) {
-            final byte[] ps = AsciiStringUtil.unsafeEncode(peerStr);
-            Bits.putShort(content, pos, (short) peerStr.length());
-            System.arraycopy(ps, 0, content, pos + 2, ps.length);
-            pos += 2 + ps.length;
+        int pos = LogEntryV1CodecFactory.PAYLOAD_OFFSET;
+
+        pos = writeLong(typeNumber, content, pos);
+        pos = writeLong(index, content, pos);
+        pos = writeLong(term, content, pos);
+
+        Bits.putLong(content, pos, log.getChecksum());
+        pos += Long.BYTES;
+
+        if (type != EntryType.ENTRY_TYPE_DATA) {
+            pos = writeNodesList(pos, content, peerStrs);
+
+            pos = writeNodesList(pos, content, oldPeerStrs);
+
+            pos = writeNodesList(pos, content, learnerStrs);
+
+            pos = writeNodesList(pos, content, oldLearnerStrs);
         }
-        // old peers
-        // old peers count
-        Bits.putInt(content, pos, oldPeerCount);
-        pos += 4;
-        for (final String peerStr : oldPeerStrs) {
-            final byte[] ps = AsciiStringUtil.unsafeEncode(peerStr);
-            Bits.putShort(content, pos, (short) peerStr.length());
-            System.arraycopy(ps, 0, content, pos + 2, ps.length);
-            pos += 2 + ps.length;
+
+        if (type != EntryType.ENTRY_TYPE_CONFIGURATION && data != null) {
+            System.arraycopy(data.array(), data.position(), content, pos, data.remaining());
         }
-        // learners
-        // learners count
-        Bits.putInt(content, pos, learnerCount);
-        pos += 4;
-        for (final String peerStr : learnerStrs) {
-            final byte[] ps = AsciiStringUtil.unsafeEncode(peerStr);
-            Bits.putShort(content, pos, (short) peerStr.length());
-            System.arraycopy(ps, 0, content, pos + 2, ps.length);
-            pos += 2 + ps.length;
+
+        return content;
+    }
+
+    private static int nodesListSizeInBytes(@Nullable List<PeerId> nodes, List<String> nodeStrs) {
+        int size = 0;
+
+        if (nodes != null) {
+            for (PeerId node : nodes) {
+                String nodeStr = node.toString();
+
+                nodeStrs.add(nodeStr);
+                size += 2 + nodeStr.length();
+            }
         }
-        // old learners
-        // old learners count
-        Bits.putInt(content, pos, oldLearnerCount);
-        pos += 4;
-        for (final String peerStr : oldLearnerStrs) {
-            final byte[] ps = AsciiStringUtil.unsafeEncode(peerStr);
-            Bits.putShort(content, pos, (short) peerStr.length());
-            System.arraycopy(ps, 0, content, pos + 2, ps.length);
-            pos += 2 + ps.length;
+
+        return size + sizeInBytes(nodeStrs.size());
+    }
+
+    private static int writeNodesList(int pos, byte[] content, List<String> nodeStrs) {
+        pos = writeLong(nodeStrs.size(), content, pos);
+
+        for (String nodeStr : nodeStrs) {
+            int length = nodeStr.length();
+
+            Bits.putShort(content, pos, (short) length);
+            pos += 2;
+
+            AsciiStringUtil.unsafeEncode(nodeStr, content, pos);
+            pos += length;
         }
-        // data
-        if (data != null) {
-            System.arraycopy(data.array(), data.position(), content, pos, data.remaining());
+
+        return pos;
+    }
+
+    // Based on DirectByteBufferStreamImplV1.
+    private static int writeLong(long val, byte[] out, int pos) {
+        while ((val & 0xFFFF_FFFF_FFFF_FF80L) != 0) {
+            byte b = (byte) (val | 0x80);
+
+            out[pos++] = b;
+
+            val >>>= 7;
         }
 
-        return content;
+        out[pos++] = (byte) val;
+
+        return pos;
+    }
+
+    /**
+     * Returns the number of bytes, required by the {@link #writeLong(long, byte[], int)} to write the value.
+    */
+    private static int sizeInBytes(long val) {
+        if (val >= 0) {
+            if (val < (1L << 7)) {
+                return 1;
+            } else if (val < (1L << 14)) {
+                return 2;
+            } else if (val < (1L << 21)) {
+                return 3;
+            } else if (val < (1L << 28)) {
+                return 4;
+            } else if (val < (1L << 35)) {
+                return 5;
+            } else if (val < (1L << 42)) {
+                return 6;
+            } else if (val < (1L << 49)) {
+                return 7;
+            } else if (val < (1L << 56)) {
+                return 8;
+            } else {
+                return 9;
+            }
+        } else {
+            return 10;
+        }
     }

Review Comment:
   ```suggestion
       private static int sizeInBytes(long val) {
           if (val >= 0) {
               int leadingZeros = Long.numberOfLeadingZeros(val);
               int bits = 64 - leadingZeros;
               return (bits + 6) / 7;
           } else {
               return 10;
           }
       }
   ```



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [ignite-3] ibessonov commented on a diff in pull request #2249: IGNITE-19828 Optimize RAFT log encoder/decoder data format

Posted by "ibessonov (via GitHub)" <gi...@apache.org>.
ibessonov commented on code in PR #2249:
URL: https://github.com/apache/ignite-3/pull/2249#discussion_r1243598274


##########
modules/raft/src/main/java/org/apache/ignite/raft/jraft/entity/codec/v1/V1Decoder.java:
##########
@@ -51,98 +52,127 @@ public LogEntry decode(final byte[] content) {
         return log;
     }
 
+    // Refactored to look closer to Ignites code style.
     public void decode(final LogEntry log, final byte[] content) {
-        // 1-5 type
-        final int iType = Bits.getInt(content, 1);
-        log.setType(EnumOutter.EntryType.forNumber(iType));
-        // 5-13 index
-        // 13-21 term
-        final long index = Bits.getLong(content, 5);
-        final long term = Bits.getLong(content, 13);
+        var reader = new Reader(content);

Review Comment:
   Ok, I'll explain data format in documentation of the _factory_



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [ignite-3] ibessonov commented on a diff in pull request #2249: IGNITE-19828 Optimize RAFT log encoder/decoder data format

Posted by "ibessonov (via GitHub)" <gi...@apache.org>.
ibessonov commented on code in PR #2249:
URL: https://github.com/apache/ignite-3/pull/2249#discussion_r1243621688


##########
modules/raft/src/main/java/org/apache/ignite/raft/jraft/entity/codec/v1/V1Encoder.java:
##########
@@ -46,130 +48,137 @@ public byte[] encode(final LogEntry log) {
         List<PeerId> oldLearners = log.getOldLearners();
         ByteBuffer data = log.getData();
 
-        // magic number 1 byte
-        int totalLen = 1;
-        final int iType = type.getNumber();
-        final long index = id.getIndex();
-        final long term = id.getTerm();
-        // type(4) + index(8) + term(8) + checksum(8)
-        totalLen += 4 + 8 + 8 + 8;
-        int peerCount = 0;
-        // peer count
-        totalLen += 4;
-        final List<String> peerStrs = new ArrayList<>();
-        if (peers != null) {
-            peerCount = peers.size();
-            for (final PeerId peer : peers) {
-                final String peerStr = peer.toString();
-                // peer len (short in 2 bytes)
-                // peer str
-                totalLen += 2 + peerStr.length();
-                peerStrs.add(peerStr);
-            }
-        }
-        int oldPeerCount = 0;
-        // old peer count
-        totalLen += 4;
-        final List<String> oldPeerStrs = new ArrayList<>();
-        if (oldPeers != null) {
-            oldPeerCount = oldPeers.size();
-            for (final PeerId peer : oldPeers) {
-                final String peerStr = peer.toString();
-                // peer len (short in 2 bytes)
-                // peer str
-                totalLen += 2 + peerStr.length();
-                oldPeerStrs.add(peerStr);
-            }
-        }
-        int learnerCount = 0;
-        // peer count
-        totalLen += 4;
-        final List<String> learnerStrs = new ArrayList<>();
-        if (learners != null) {
-            learnerCount = learners.size();
-            for (final PeerId learner : learners) {
-                final String learnerStr = learner.toString();
-                // learner len (short in 2 bytes)
-                // learner str
-                totalLen += 2 + learnerStr.length();
-                learnerStrs.add(learnerStr);
-            }
-        }
-        int oldLearnerCount = 0;
-        // old peer count
-        totalLen += 4;
-        final List<String> oldLearnerStrs = new ArrayList<>();
-        if (oldLearners != null) {
-            oldLearnerCount = oldLearners.size();
-            for (final PeerId oldLearner : oldLearners) {
-                final String learnerStr = oldLearner.toString();
-                // oldLearner len (short in 2 bytes)
-                // oldLearner str
-                totalLen += 2 + learnerStr.length();
-                oldLearnerStrs.add(learnerStr);
-            }
+        int totalLen = LogEntryV1CodecFactory.PAYLOAD_OFFSET;
+        int typeNumber = type.getNumber();
+        long index = id.getIndex();
+        long term = id.getTerm();
+
+        totalLen += sizeInBytes(typeNumber) + sizeInBytes(index) + sizeInBytes(term) + 8;
+
+        List<String> peerStrs = null;
+        List<String> oldPeerStrs = null;
+        List<String> learnerStrs = null;
+        List<String> oldLearnerStrs = null;
+
+        if (type != EntryType.ENTRY_TYPE_DATA) {
+            peerStrs = new ArrayList<>();
+            totalLen += nodesListSizeInBytes(peers, peerStrs);
+
+            oldPeerStrs = new ArrayList<>();
+            totalLen += nodesListSizeInBytes(oldPeers, oldPeerStrs);
+
+            learnerStrs = new ArrayList<>();
+            totalLen += nodesListSizeInBytes(learners, learnerStrs);
+
+            oldLearnerStrs = new ArrayList<>();
+            totalLen += nodesListSizeInBytes(oldLearners, oldLearnerStrs);
         }
 
-        final int bodyLen = data != null ? data.remaining() : 0;
-        totalLen += bodyLen;
+        if (type != EntryType.ENTRY_TYPE_CONFIGURATION) {
+            int bodyLen = data != null ? data.remaining() : 0;
+            totalLen += bodyLen;
+        }
 
-        final byte[] content = new byte[totalLen];
-        // {0} magic
+        byte[] content = new byte[totalLen];
         content[0] = LogEntryV1CodecFactory.MAGIC;
-        // 1-5 type
-        Bits.putInt(content, 1, iType);
-        // 5-13 index
-        Bits.putLong(content, 5, index);
-        // 13-21 term
-        Bits.putLong(content, 13, term);
-        // checksum
-        Bits.putLong(content, 21, log.getChecksum());
-
-        // peers
-        // 21-25 peer count
-        Bits.putInt(content, 29, peerCount);
-        int pos = 33;
-        for (final String peerStr : peerStrs) {
-            final byte[] ps = AsciiStringUtil.unsafeEncode(peerStr);
-            Bits.putShort(content, pos, (short) peerStr.length());
-            System.arraycopy(ps, 0, content, pos + 2, ps.length);
-            pos += 2 + ps.length;
+        int pos = LogEntryV1CodecFactory.PAYLOAD_OFFSET;
+
+        pos = writeLong(typeNumber, content, pos);
+        pos = writeLong(index, content, pos);
+        pos = writeLong(term, content, pos);
+
+        Bits.putLong(content, pos, log.getChecksum());
+        pos += Long.BYTES;
+
+        if (type != EntryType.ENTRY_TYPE_DATA) {
+            pos = writeNodesList(pos, content, peerStrs);
+
+            pos = writeNodesList(pos, content, oldPeerStrs);
+
+            pos = writeNodesList(pos, content, learnerStrs);
+
+            pos = writeNodesList(pos, content, oldLearnerStrs);
         }
-        // old peers
-        // old peers count
-        Bits.putInt(content, pos, oldPeerCount);
-        pos += 4;
-        for (final String peerStr : oldPeerStrs) {
-            final byte[] ps = AsciiStringUtil.unsafeEncode(peerStr);
-            Bits.putShort(content, pos, (short) peerStr.length());
-            System.arraycopy(ps, 0, content, pos + 2, ps.length);
-            pos += 2 + ps.length;
+
+        if (type != EntryType.ENTRY_TYPE_CONFIGURATION && data != null) {
+            System.arraycopy(data.array(), data.position(), content, pos, data.remaining());
         }
-        // learners
-        // learners count
-        Bits.putInt(content, pos, learnerCount);
-        pos += 4;
-        for (final String peerStr : learnerStrs) {
-            final byte[] ps = AsciiStringUtil.unsafeEncode(peerStr);
-            Bits.putShort(content, pos, (short) peerStr.length());
-            System.arraycopy(ps, 0, content, pos + 2, ps.length);
-            pos += 2 + ps.length;
+
+        return content;
+    }
+
+    private static int nodesListSizeInBytes(@Nullable List<PeerId> nodes, List<String> nodeStrs) {
+        int size = 0;
+
+        if (nodes != null) {
+            for (PeerId node : nodes) {
+                String nodeStr = node.toString();
+
+                nodeStrs.add(nodeStr);
+                size += 2 + nodeStr.length();
+            }
         }
-        // old learners
-        // old learners count
-        Bits.putInt(content, pos, oldLearnerCount);
-        pos += 4;
-        for (final String peerStr : oldLearnerStrs) {
-            final byte[] ps = AsciiStringUtil.unsafeEncode(peerStr);
-            Bits.putShort(content, pos, (short) peerStr.length());
-            System.arraycopy(ps, 0, content, pos + 2, ps.length);
-            pos += 2 + ps.length;
+
+        return size + sizeInBytes(nodeStrs.size());
+    }
+
+    private static int writeNodesList(int pos, byte[] content, List<String> nodeStrs) {
+        pos = writeLong(nodeStrs.size(), content, pos);
+
+        for (String nodeStr : nodeStrs) {
+            int length = nodeStr.length();
+
+            Bits.putShort(content, pos, (short) length);
+            pos += 2;
+
+            AsciiStringUtil.unsafeEncode(nodeStr, content, pos);
+            pos += length;
         }
-        // data
-        if (data != null) {
-            System.arraycopy(data.array(), data.position(), content, pos, data.remaining());
+
+        return pos;
+    }
+
+    // Based on DirectByteBufferStreamImplV1.
+    private static int writeLong(long val, byte[] out, int pos) {
+        while ((val & 0xFFFF_FFFF_FFFF_FF80L) != 0) {
+            byte b = (byte) (val | 0x80);
+
+            out[pos++] = b;
+
+            val >>>= 7;
         }
 
-        return content;
+        out[pos++] = (byte) val;
+
+        return pos;
+    }
+
+    /**
+     * Returns the number of bytes, required by the {@link #writeLong(long, byte[], int)} to write the value.
+    */
+    private static int sizeInBytes(long val) {
+        if (val >= 0) {
+            if (val < (1L << 7)) {
+                return 1;
+            } else if (val < (1L << 14)) {
+                return 2;
+            } else if (val < (1L << 21)) {
+                return 3;
+            } else if (val < (1L << 28)) {
+                return 4;
+            } else if (val < (1L << 35)) {
+                return 5;
+            } else if (val < (1L << 42)) {
+                return 6;
+            } else if (val < (1L << 49)) {
+                return 7;
+            } else if (val < (1L << 56)) {
+                return 8;
+            } else {
+                return 9;
+            }
+        } else {
+            return 10;
+        }
     }

Review Comment:
   It's going to be slower on smaller values



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [ignite-3] ibessonov merged pull request #2249: IGNITE-19828 Optimize RAFT log encoder/decoder data format

Posted by "ibessonov (via GitHub)" <gi...@apache.org>.
ibessonov merged PR #2249:
URL: https://github.com/apache/ignite-3/pull/2249


-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [ignite-3] SammyVimes commented on a diff in pull request #2249: IGNITE-19828 Optimize RAFT log encoder/decoder data format

Posted by "SammyVimes (via GitHub)" <gi...@apache.org>.
SammyVimes commented on code in PR #2249:
URL: https://github.com/apache/ignite-3/pull/2249#discussion_r1243549222


##########
modules/network/src/main/java/org/apache/ignite/internal/network/direct/stream/DirectByteBufferStreamImplV1.java:
##########
@@ -188,7 +188,7 @@ public void setBuffer(ByteBuffer buf) {
             this.buf = buf;
 
             heapArr = buf.isDirect() ? null : buf.array();
-            baseOff = buf.isDirect() ? GridUnsafe.bufferAddress(buf) : BYTE_ARR_OFF;
+            baseOff = buf.isDirect() ? GridUnsafe.bufferAddress(buf) : BYTE_ARR_OFF + buf.arrayOffset();

Review Comment:
   Probably position should be respected too then (for both types)



##########
modules/raft/src/main/java/org/apache/ignite/raft/jraft/entity/codec/v1/V1Decoder.java:
##########
@@ -51,98 +52,127 @@ public LogEntry decode(final byte[] content) {
         return log;
     }
 
+    // Refactored to look closer to Ignites code style.
     public void decode(final LogEntry log, final byte[] content) {
-        // 1-5 type
-        final int iType = Bits.getInt(content, 1);
-        log.setType(EnumOutter.EntryType.forNumber(iType));
-        // 5-13 index
-        // 13-21 term
-        final long index = Bits.getLong(content, 5);
-        final long term = Bits.getLong(content, 13);
+        var reader = new Reader(content);

Review Comment:
   Probably some of the comments should be retained. Or maybe we can describe the format of the entry here (like we do in storages)



##########
modules/raft/src/main/java/org/apache/ignite/raft/jraft/entity/codec/v1/V1Encoder.java:
##########
@@ -46,130 +48,137 @@ public byte[] encode(final LogEntry log) {
         List<PeerId> oldLearners = log.getOldLearners();
         ByteBuffer data = log.getData();
 
-        // magic number 1 byte
-        int totalLen = 1;
-        final int iType = type.getNumber();
-        final long index = id.getIndex();
-        final long term = id.getTerm();
-        // type(4) + index(8) + term(8) + checksum(8)
-        totalLen += 4 + 8 + 8 + 8;
-        int peerCount = 0;
-        // peer count
-        totalLen += 4;
-        final List<String> peerStrs = new ArrayList<>();
-        if (peers != null) {
-            peerCount = peers.size();
-            for (final PeerId peer : peers) {
-                final String peerStr = peer.toString();
-                // peer len (short in 2 bytes)
-                // peer str
-                totalLen += 2 + peerStr.length();
-                peerStrs.add(peerStr);
-            }
-        }
-        int oldPeerCount = 0;
-        // old peer count
-        totalLen += 4;
-        final List<String> oldPeerStrs = new ArrayList<>();
-        if (oldPeers != null) {
-            oldPeerCount = oldPeers.size();
-            for (final PeerId peer : oldPeers) {
-                final String peerStr = peer.toString();
-                // peer len (short in 2 bytes)
-                // peer str
-                totalLen += 2 + peerStr.length();
-                oldPeerStrs.add(peerStr);
-            }
-        }
-        int learnerCount = 0;
-        // peer count
-        totalLen += 4;
-        final List<String> learnerStrs = new ArrayList<>();
-        if (learners != null) {
-            learnerCount = learners.size();
-            for (final PeerId learner : learners) {
-                final String learnerStr = learner.toString();
-                // learner len (short in 2 bytes)
-                // learner str
-                totalLen += 2 + learnerStr.length();
-                learnerStrs.add(learnerStr);
-            }
-        }
-        int oldLearnerCount = 0;
-        // old peer count
-        totalLen += 4;
-        final List<String> oldLearnerStrs = new ArrayList<>();
-        if (oldLearners != null) {
-            oldLearnerCount = oldLearners.size();
-            for (final PeerId oldLearner : oldLearners) {
-                final String learnerStr = oldLearner.toString();
-                // oldLearner len (short in 2 bytes)
-                // oldLearner str
-                totalLen += 2 + learnerStr.length();
-                oldLearnerStrs.add(learnerStr);
-            }
+        int totalLen = LogEntryV1CodecFactory.PAYLOAD_OFFSET;

Review Comment:
   Same about comments/entry format



##########
modules/raft/src/main/java/org/apache/ignite/raft/jraft/entity/codec/v1/V1Decoder.java:
##########
@@ -51,98 +52,127 @@ public LogEntry decode(final byte[] content) {
         return log;
     }
 
+    // Refactored to look closer to Ignites code style.
     public void decode(final LogEntry log, final byte[] content) {
-        // 1-5 type
-        final int iType = Bits.getInt(content, 1);
-        log.setType(EnumOutter.EntryType.forNumber(iType));
-        // 5-13 index
-        // 13-21 term
-        final long index = Bits.getLong(content, 5);
-        final long term = Bits.getLong(content, 13);
+        var reader = new Reader(content);
+        reader.pos = LogEntryV1CodecFactory.PAYLOAD_OFFSET;
+
+        int typeNumber = (int)reader.readLong();
+        EnumOutter.EntryType type = Objects.requireNonNull(EnumOutter.EntryType.forNumber(typeNumber));
+        log.setType(type);
+
+        long index = reader.readLong();
+        long term = reader.readLong();
         log.setId(new LogId(index, term));
-        // 21-29 checksum
-        log.setChecksum(Bits.getLong(content, 21));
-        // 29-33 peer count
-        int peerCount = Bits.getInt(content, 29);
-        // peers
-        int pos = 33;
-        if (peerCount > 0) {
-            List<PeerId> peers = new ArrayList<>(peerCount);
-            while (peerCount-- > 0) {
-                final short len = Bits.getShort(content, pos);
-                final byte[] bs = new byte[len];
-                System.arraycopy(content, pos + 2, bs, 0, len);
-                // peer len (short in 2 bytes)
-                // peer str
-                pos += 2 + len;
-                final PeerId peer = new PeerId();
-                peer.parse(AsciiStringUtil.unsafeDecode(bs));
-                peers.add(peer);
+
+        long checksum = Bits.getLong(content, reader.pos);
+        log.setChecksum(checksum);
+
+        int pos = reader.pos + Long.BYTES;
+
+        // Peers and learners.
+        if (type != EnumOutter.EntryType.ENTRY_TYPE_DATA) {
+            reader.pos = pos;
+            int peerCount = (int)reader.readLong();
+            pos = reader.pos;
+            if (peerCount > 0) {
+                List<PeerId> peers = new ArrayList<>(peerCount);
+
+                pos = readNodesList(pos, content, peerCount, peers);
+
+                log.setPeers(peers);
             }
-            log.setPeers(peers);
-        }
-        // old peers
-        int oldPeerCount = Bits.getInt(content, pos);
-        pos += 4;
-        if (oldPeerCount > 0) {
-            List<PeerId> oldPeers = new ArrayList<>(oldPeerCount);
-            while (oldPeerCount-- > 0) {
-                final short len = Bits.getShort(content, pos);
-                final byte[] bs = new byte[len];
-                System.arraycopy(content, pos + 2, bs, 0, len);
-                // peer len (short in 2 bytes)
-                // peer str
-                pos += 2 + len;
-                final PeerId peer = new PeerId();
-                peer.parse(AsciiStringUtil.unsafeDecode(bs));
-                oldPeers.add(peer);
+
+            reader.pos = pos;
+            int oldPeerCount = (int)reader.readLong();
+            pos = reader.pos;
+            if (oldPeerCount > 0) {
+                List<PeerId> oldPeers = new ArrayList<>(oldPeerCount);
+
+                pos = readNodesList(pos, content, oldPeerCount, oldPeers);
+
+                log.setOldPeers(oldPeers);
             }
-            log.setOldPeers(oldPeers);
-        }
-        // learners
-        int learnersCount = Bits.getInt(content, pos);
-        pos += 4;
-        if (learnersCount > 0) {
-            List<PeerId> learners = new ArrayList<>(learnersCount);
-            while (learnersCount-- > 0) {
-                final short len = Bits.getShort(content, pos);
-                final byte[] bs = new byte[len];
-                System.arraycopy(content, pos + 2, bs, 0, len);
-                // peer len (short in 2 bytes)
-                // peer str
-                pos += 2 + len;
-                final PeerId peer = new PeerId();
-                peer.parse(AsciiStringUtil.unsafeDecode(bs));
-                learners.add(peer);
+
+            reader.pos = pos;
+            int learnersCount = (int)reader.readLong();
+            pos = reader.pos;
+            if (learnersCount > 0) {
+                List<PeerId> learners = new ArrayList<>(learnersCount);
+
+                pos = readNodesList(pos, content, learnersCount, learners);
+
+                log.setLearners(learners);
+            }
+
+            reader.pos = pos;
+            int oldLearnersCount = (int)reader.readLong();
+            pos = reader.pos;
+            if (oldLearnersCount > 0) {
+                List<PeerId> oldLearners = new ArrayList<>(oldLearnersCount);
+
+                pos = readNodesList(pos, content, oldLearnersCount, oldLearners);
+
+                log.setOldLearners(oldLearners);
             }
-            log.setLearners(learners);
         }
-        // old learners
-        int oldLearnersCount = Bits.getInt(content, pos);
-        pos += 4;
-        if (oldLearnersCount > 0) {
-            List<PeerId> oldLearners = new ArrayList<>(oldLearnersCount);
-            while (oldLearnersCount-- > 0) {
-                final short len = Bits.getShort(content, pos);
-                final byte[] bs = new byte[len];
-                System.arraycopy(content, pos + 2, bs, 0, len);
-                // peer len (short in 2 bytes)
-                // peer str
-                pos += 2 + len;
-                final PeerId peer = new PeerId();
-                peer.parse(AsciiStringUtil.unsafeDecode(bs));
-                oldLearners.add(peer);
+
+        // Data.
+        if (type != EnumOutter.EntryType.ENTRY_TYPE_CONFIGURATION) {
+            if (content.length > pos) {
+                int len = content.length - pos;
+
+                ByteBuffer data = ByteBuffer.wrap(content, pos, len).slice();
+
+                log.setData(data);
             }
-            log.setOldLearners(oldLearners);
         }
+    }
+
+    private static int readNodesList(int pos, byte[] content, int count, List<PeerId> nodes) {
+        while (count --> 0) {

Review Comment:
   ```suggestion
           while (count-- > 0) {
   ```



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org