quic: Avoid bytes for VariableLengthInteger #13
Conversation
analyzer/QUIC.spicy
Outdated
| byte1: uint8 if ( self.bytes_to_parse > 1 ) { | ||
| self.result = (self.result << 8) | $$; | ||
| } | ||
|
|
||
| byte2: uint8 if ( self.bytes_to_parse > 2 ) { | ||
| self.result = (self.result << 8) | $$; | ||
| } | ||
|
|
||
| byte3: uint8 if ( self.bytes_to_parse > 3 ) { | ||
| self.result = (self.result << 8) | $$; | ||
| } |
There was a problem hiding this comment.
You could avoid storing these uninteresting values and the repetition here, e.g.,
| byte1: uint8 if ( self.bytes_to_parse > 1 ) { | |
| self.result = (self.result << 8) | $$; | |
| } | |
| byte2: uint8 if ( self.bytes_to_parse > 2 ) { | |
| self.result = (self.result << 8) | $$; | |
| } | |
| byte3: uint8 if ( self.bytes_to_parse > 3 ) { | |
| self.result = (self.result << 8) | $$; | |
| } | |
| : uint8[self.bytes_to_parse - 1] if (self.bytes_to_parse > 0) foreach { | |
| self.result = (self.result << 8) | $$; | |
| } |
There was a problem hiding this comment.
You could make this even simpler if you made self.bytes_to_parse zero-based, e.g., on init self.bytes_to_parse = 2**((0xC0 & $$) >> 6) - 1 (probably needs a check that the value is not already zero to avoid underflow). You could then parse the remaining bytes with
: uint8[self.bytes_to_parse] foreach {
self.result = (self.result << 8) | $$;
}There was a problem hiding this comment.
Thanks, nice!
I like the explicit if of the first proposal and will go with that. Also fits well with the example in the RFC. Think it needs to be if ( self.bytes_to_parse > 1) though.
There was a problem hiding this comment.
Using if ( self.bytes_to_parse > 1) would short-circuit even setting up parsing (for nothing), so it definitely wouldn't hurt.
Allocation of bytes objects due to parsing and usage of pack and the invocation of to_uint() showed significantly in profiles (3.3% sample matches). Switch to a more procedural approach to avoid the allocation overhead.
awelzel
force-pushed
the
topic/awelzel/no-bytes-in-variable-length-integer
branch
from
6a67961 to
2af9f65
Compare
Allocation of bytes objects due to parsing and usage of pack and the invocation of to_uint() showed significantly in profiles (3.3% sample matches). Switch to a more procedural approach to avoid the allocation overhead. From zeek/spicy-quic/pull/13
Allocation of bytes objects due to parsing and usage of pack and the invocation of to_uint() showed significantly in profiles (3.3% sample matches). Switch to a more procedural approach to avoid the allocation overhead.