Repository navigation
perf(server): cut hot-path CPU 13% and route all field access through accessors - #35
Merged
Merged
Conversation
- Updated documentation comments across various modules to enhance clarity and consistency, particularly in trait and function signatures. - Introduced a thread-local buffer pool for efficient read buffer management in the stream module, reducing allocation overhead for keep-alive connections. - Implemented a `PooledReader` struct to manage buffered reads from TCP streams, optimizing performance by reusing buffers. - Enhanced the request and response handling in the stream module to utilize the new buffered reader, improving efficiency in parsing HTTP requests and responses. - Added a `reset` method to both `Request` and `Response` structs to allow for reusing existing allocations during keep-alive connections.
Per project rule, field accessors come from the lombok Data/Getter/ GetterMut macros wherever the macro can generate them: - Proxy, ProxyTunnelStream, SyncProxyTunnelStream, Tmp, RequestBuilder: derive(Data), hand-written get_*/set_* removed, call sites renamed to lombok names (get_mut_*/get_*). - PooledReader: derive(Data) verified working on the lifetime struct with the &'a mut TcpStream field; hand-written accessor block removed. - HttpRequest: derive(GetterMut) with #[get_mut(skip)] on all fields except headers, generating only get_mut_headers; the existing hand-written accessor API (get_method -> Method, set_url(impl Into), etc.) is the published crates.io contract and is kept as-is since a full Data derive would collide with it. - Body: derive(Getter) for get_bytes().
vshengbro
commented
Sep 27, 2026
Comment on lines
+3
to
+4
| thread_local! { | ||
| static READ_BUFFER_POOL: RefCell<Vec<Vec<u8>>> = const { RefCell::new(Vec::new()) }; |
Member
Author
There was a problem hiding this comment.
Done — the thread_local READ_BUFFER_POOL now lives in type/src/stream/static.rs (wired via mod.rs with pub(crate) use), per the workspace file-layout convention.
vshengbro
commented
Sep 27, 2026
| /// # Returns | ||
| /// | ||
| /// - `Self`: The reader with an empty valid-data region. | ||
| pub(crate) fn new(stream: &'a mut TcpStream, buffer: Vec<u8>) -> Self { |
Member
Author
There was a problem hiding this comment.
delete and use lombok New macro
Member
Author
There was a problem hiding this comment.
Done — the hand-written new is deleted; PooledReader now derives lombok New with #[new(skip)] on start/end (default-initialized to 0), so the 2-arg constructor call site is unchanged.
vshengbro
commented
Sep 27, 2026
| Request::fill_http_querys(query, request.get_mut_querys()); | ||
| let path_slice: &str = Request::get_http_path(path, query_index, hash_index); | ||
| request.get_mut_path().push_str(path_slice); | ||
| let Request { headers, host, .. } = request; |
Member
Author
There was a problem hiding this comment.
replace request.get_headers() and request.get_host() by lobmok Data macro
Member
Author
There was a problem hiding this comment.
Done — the destructuring at the call site is gone. Request::get_http_headers is now an &mut self method (request.get_http_headers(&mut reader, &config)), so the parser fills headers/host internally. Note: the two fields could not be passed as request.get_mut_headers() + request.get_mut_host() in one argument list — that is two simultaneous &mut borrows of the same request (E0499), which is why the access moved inside the method.
added 2 commits
September 27, 2026 10:13
lombok emits accessors as pub by default; fields that are crate internals (tunnel stream innards, Tmp redirect state, the builder's in-progress request, PooledReader buffers) now carry #[get/get_mut/set(pub(crate))] so the macro-generated surface matches the field exposure instead of widening it. Structs whose fields are already pub (Proxy, HttpRequest headers, Body) keep pub accessors.
- Move the READ_BUFFER_POOL thread_local into the module's static.rs per the workspace file-layout convention. - Drop the hand-written PooledReader::new in favor of the lombok New derive (#[new(skip)] on start/end default-initializes them to 0). - Turn Request::get_http_headers into an &mut self method so the call site no longer destructures the request; the parser fills the headers map and host string through the struct's own fields (the two mutable borrows cannot coexist as two macro accessor calls in one argument list, so the access moved inside the method).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two stacked changes on the HTTP hot path:
PooledReaderbuffer pool + fill-style request parsing + keep-alive request/response object reuse (Request::reset/Response::resetpreserve capacity,mem::takeswap-in inhandle_http_requests). Eliminates per-request allocations of the read buffer, theRequest/Responsestructs and their inner maps/vecs, plusrequest.clone()andbody.clone()on the dispatch path.self.field/this.fieldaccess in production code. All field access now goes through lombokData-generated or hand-written accessors (PooledReaderaccessor set,Proxy/ProxyTunnelStream/SyncProxyTunnelStream/RequestBuilder/Tmphand-written sets in http-request, lombokget_mut_*/set_*in http-type). Where two mutable field borrows are needed in one expression, struct destructuring is used instead (borrowck-safe, rule-compliant).Verification
verify_no_self_field_access.py: 116 → 0 violationsverify_no_import_rename.py: 0 violations (§6.5)cargo check --workspace --offline: 0 errorscargo clippy --workspace --all-targets --offline: 0 warningscargo test -p http-type -p hyperlane-core -p http-request --offline: 377 passed, 0 failedNotes