Optimize event serialization with pre-allocated buffer #2794
Conversation
- Added event_buffer field to LlmpEventManager - Used to_slice instead of to_allocvec - Pre-allocated buffer size is 4KB Fixes AFLplusplus#1082
Yeah it's difficult... imo i don't think using pre-allocated buffer makes it significantly efficient, but on the other hand it makes the code unnecessarily complex. |
|
In the error case, can't we just use |
|
sounds good |
Also combined the shared logic between compressed & uncompressed event firing while keeping the same behavior
libafl/src/events/llmp/mgr.rs
Outdated
| @@ -199,6 +201,7 @@ impl<EMH> LlmpEventManagerBuilder<EMH> { | |||
| time_ref, | |||
| phantom: PhantomData, | |||
| custom_buf_handlers: vec![], | |||
| event_buffer: Vec::with_capacity(1024 * 4), | |||
There was a problem hiding this comment.
Can we make the initial size a const?
libafl/src/events/llmp/mgr.rs
Outdated
| flags | LLMP_FLAG_COMPRESSED, | ||
| &comp_buf, | ||
| )?; | ||
| self.event_buffer.clear(); |
| .compressor | ||
| .maybe_compress(&self.event_buffer[..written_len]) | ||
| { | ||
| Some(comp_buf) => { |
There was a problem hiding this comment.
technically this still allocates, so we have future room for improvements
There was a problem hiding this comment.
Sorry, I don't understand what you mean exactly. Are you referring to the allocation happening in maybe_compress function?
There was a problem hiding this comment.
Yes in theory this also could use a preallocated buffer
Also removed the unnecessary event_buffer.clear(), since we are already resizing it
|
Thank you! :) |
Fixes #1082
I tested the changes with libpng fuzzer, and noticed quite a significant performance improvement, though I haven't done any formal benchmarking.
Current Implementation Issue:
The fuzzer panics after 4-5 minutes of execution with a "SerializeBuffer full" error. While increasing the initial buffer size could help, it might not be the most elegant solution(?)
Questions/Feedback Needed:
to_allocvecwhen the pre-allocated buffer is full?