From b8dfbae75d8e13b78d3a7d5db48de2f5e262cd87 Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Sat, 9 Mar 2024 16:03:10 +0100 Subject: [PATCH] Improve entry crc32 computation We have a codepath difference here, where we use either a ByteBuffer or a raw array to compute CRC32. MappedJournalSegmentWriter does not make it immediately clear we use this buffer twice -- once for CRC32 and once for deserialization. Move acquisition of slice just after we have read the expected CRC32, so it is clear it is something we would be doing even if there were no checksum involved. Mirror the same in FileChannelJournalSegmentWriter, as this will allow us to further consolidate the code and stop mucking with memory.limit(), which is causing us to invalidate our previously-set mark. JIRA: CONTROLLER-2095 Change-Id: I355bd97cd8acb4f5d9d91310de97ecb2cbd70282 Signed-off-by: Robert Varga --- .../journal/FileChannelJournalSegmentWriter.java | 6 +++++- .../storage/journal/MappedJournalSegmentWriter.java | 12 +++++++----- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/atomix-storage/src/main/java/io/atomix/storage/journal/FileChannelJournalSegmentWriter.java b/atomix-storage/src/main/java/io/atomix/storage/journal/FileChannelJournalSegmentWriter.java index 5bd5212c2e..39d2363676 100644 --- a/atomix-storage/src/main/java/io/atomix/storage/journal/FileChannelJournalSegmentWriter.java +++ b/atomix-storage/src/main/java/io/atomix/storage/journal/FileChannelJournalSegmentWriter.java @@ -95,9 +95,13 @@ class FileChannelJournalSegmentWriter implements JournalWriter { // Read the checksum of the entry. final long checksum = memory.getInt() & 0xFFFFFFFFL; + // Slice off the entry's bytes + final ByteBuffer entryBytes = memory.slice(); + entryBytes.limit(length); + // Compute the checksum for the entry bytes. final CRC32 crc32 = new CRC32(); - crc32.update(memory.array(), memory.position(), length); + crc32.update(entryBytes); // If the stored checksum does not equal the computed checksum, do not proceed further if (checksum != crc32.getValue()) { diff --git a/atomix-storage/src/main/java/io/atomix/storage/journal/MappedJournalSegmentWriter.java b/atomix-storage/src/main/java/io/atomix/storage/journal/MappedJournalSegmentWriter.java index 7ff58590d9..bc74e8b0ef 100644 --- a/atomix-storage/src/main/java/io/atomix/storage/journal/MappedJournalSegmentWriter.java +++ b/atomix-storage/src/main/java/io/atomix/storage/journal/MappedJournalSegmentWriter.java @@ -97,19 +97,21 @@ class MappedJournalSegmentWriter implements JournalWriter { // Read the checksum of the entry. final long checksum = buffer.getInt() & 0xFFFFFFFFL; + // Slice off the entry's bytes + final ByteBuffer entryBytes = buffer.slice(); + entryBytes.limit(length); + // Compute the checksum for the entry bytes. final CRC32 crc32 = new CRC32(); - ByteBuffer slice = buffer.slice(); - slice.limit(length); - crc32.update(slice); + crc32.update(entryBytes); // If the stored checksum does not equal the computed checksum, do not proceed further if (checksum != crc32.getValue()) { break; } - slice.rewind(); - final E entry = namespace.deserialize(slice); + entryBytes.rewind(); + final E entry = namespace.deserialize(entryBytes); lastEntry = new Indexed<>(nextIndex, entry, length); this.index.index(nextIndex, position); nextIndex++; -- 2.36.6