feat: support in memory trace compression #2504
Conversation
DavePearce
force-pushed
the
2501-feat-support-in-memory-trace-compression
branch
from
e51fa81 to
7508d07
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
arithmetization/src/main/java/net/consensys/linea/zktracer/LtTraceFile.java
Show resolved
Hide resolved
arithmetization/src/main/java/net/consensys/linea/zktracer/LtTraceFile.java
Show resolved
Hide resolved
letypequividelespoubelles
left a comment
letypequividelespoubelles
left a comment
There was a problem hiding this comment.
LGTM but I'd like someone else to review it :)
arithmetization/src/main/java/net/consensys/linea/zktracer/ZkTracer.java
Show resolved
Hide resolved
| this.name = name; | ||
| this.bitWidth = bitwidth; | ||
| this.byteWidth = byteWidth(bitwidth); | ||
| this.longMax = 1L << bitwidth; |
There was a problem hiding this comment.
Bug: Bit shift overflow for 64-bit columns
The expression 1L << bitwidth produces undefined behavior when bitwidth is 64 or greater. In Java, left shift operations use only the lower 6 bits of the shift amount for long values, so 1L << 64 becomes 1L << 0, resulting in longMax = 1. This causes the validation check if (longMax <= value) to incorrectly reject all values greater than or equal to 1 for 64-bit columns, breaking trace generation for full-width long values.
DavePearce
force-pushed
the
2501-feat-support-in-memory-trace-compression
branch
3 times, most recently
from
9572e00 to
5552977
Compare
| this.name = name; | ||
| this.bitWidth = bitwidth; | ||
| this.byteWidth = byteWidth(bitwidth); | ||
| this.longMax = 1L << bitwidth; |
There was a problem hiding this comment.
Bug: Shift overflow breaks validation for large bitwidths
The calculation longMax = 1L << bitwidth produces incorrect results when bitwidth >= 63. For bitwidth = 64, Java masks the shift to 1L << 0 = 1, causing the validation to reject nearly all values. For bitwidth = 63, the result is Long.MIN_VALUE (negative), breaking the comparison logic. This causes incorrect validation failures for columns with large bit widths.
Additional Locations (1)
This alters the way in which trace generation occurs, in that it traces are always generated fully in memory before being written to disk. This allows various possibilities. For example, we can compress the data prior to writing to disk. Or, we can use an alternative format which requires holistic information of the entire trace before being written to disk (e.g. it uses some kind of heap, etc).
Trace compression should be enabled by default, but employs a feature flag which can be used to disable it.
This updates go-corset to a version which is compatible with the trace compression code used by this PR.
DavePearce
force-pushed
the
2501-feat-support-in-memory-trace-compression
branch
from
5552977 to
0e03668
Compare
| this.name = name; | ||
| this.bitWidth = bitwidth; | ||
| this.byteWidth = byteWidth(bitwidth); | ||
| this.longMax = 1L << bitwidth; |
There was a problem hiding this comment.
Bug: Shift overflow for 64-bit columns
The calculation this.longMax = 1L << bitwidth produces incorrect results when bitwidth is 64 or greater. In Java, shift amounts for long are masked to 6 bits, so 1L << 64 becomes 1L << 0 = 1L. This causes the validation check on line 54 to incorrectly reject nearly all values for 64-bit columns, since longMax would be 1L instead of representing the maximum valid value.
There was an issue due to the maximum size of a Java byte array (~2G). Specifically, the entire trace was being put into a single byte array, and this was causing an overflow. Instead, we just write directly using output streams.
DavePearce
force-pushed
the
2501-feat-support-in-memory-trace-compression
branch
from
23c4e84 to
1270157
Compare
| this.name = name; | ||
| this.bitWidth = bitwidth; | ||
| this.byteWidth = byteWidth(bitwidth); | ||
| this.longMax = 1L << bitwidth; |
There was a problem hiding this comment.
Bug: Shift overflow breaks validation for large bitwidths
The calculation longMax = 1L << bitwidth produces incorrect results for bitwidth >= 63 due to Java's shift masking behavior. For bitwidth=63, it yields the minimum long value (negative), and for bitwidth=64, it yields 1 due to the shift amount being masked to 6 bits. This breaks the validation check if (longMax <= value), causing it to incorrectly reject valid values or accept invalid ones when using large bitwidths.
Additional Locations (1)
| this.bitWidth = bitwidth; | ||
| this.byteWidth = byteWidth(bitwidth); | ||
| this.longMax = 1L << bitwidth; | ||
| this.buffer = ByteBuffer.allocate(length * byteWidth); |
There was a problem hiding this comment.
Bug: Integer overflow in buffer size calculation
The buffer allocation ByteBuffer.allocate(length * byteWidth) can overflow when length and byteWidth are large. For example, with byteWidth=8 and length > 268435455, the multiplication exceeds Integer.MAX_VALUE, causing integer overflow. This results in either a negative value (throwing IllegalArgumentException) or a wrapped positive value (allocating an incorrectly sized buffer that causes BufferOverflowException during writes).
* support in memory trace generation This alters the way in which trace generation occurs, in that it traces are always generated fully in memory before being written to disk. This allows various possibilities. For example, we can compress the data prior to writing to disk. Or, we can use an alternative format which requires holistic information of the entire trace before being written to disk (e.g. it uses some kind of heap, etc). * add plugin cli option for trace compression Trace compression should be enabled by default, but employs a feature flag which can be used to disable it. * update go-corset to v1.1.29 This updates go-corset to a version which is compatible with the trace compression code used by this PR. * add logging and minor comments * fix out-of-memory issue There was an issue due to the maximum size of a Java byte array (~2G). Specifically, the entire trace was being put into a single byte array, and this was causing an overflow. Instead, we just write directly using output streams.
Note
Introduces an in-memory LT trace writer with optional gzip compression (configurable via CLI/config), updates file extensions/paths and tests, and bumps go-corset.
LtTraceFilethat builds columns/headers and streams toOutputStream.ZkTracer.writeToFile()now writes viaLtTraceFileand usesGZIPOutputStreamwhen filename ends with.gz; metadata construction refactored tobuildMetaData.TraceWriteracceptstraceCompressionflag and chooses.lt.gz/.lt(and temp) extensions; updates path generation and temp write workflow.TracesEndpointConfigurationandTracesEndpointCliOptionswithtraceCompression(default true) and plumb intoGenerateConflatedTracesV2..lt.gz).v1.1.29in GitHub Action.Written by Cursor Bugbot for commit 1270157. This will update automatically on new commits. Configure here.