Optimize per-request critical path performance #365
Conversation
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
- Replace istringstream-based string_split with manual loop (string_utilities.cpp) - Fix http_unescape: parse hex chars directly without substr allocations (http_utils.cpp) - Flatten unique_ptr<string> members in modded_request to plain std::string, eliminating 2 heap allocations per request (modded_request.hpp, webserver.cpp) - Fix strcmp/strcasecmp inconsistency: all HTTP method dispatch now uses case-sensitive strcmp per RFC 7230 (webserver.cpp) - Optimize set_method: skip unnecessary copy+uppercase since MHD provides method strings in uppercase per spec (http_request.cpp) - Cache get_path_pieces() result on first access to avoid re-tokenization on every call (http_request.hpp) - Separate exact vs parameterized routes at registration time; only scan parameterized routes on regex fallback (webserver.hpp, webserver.cpp) - Add shared LRU cache (256 entries) for URL-to-endpoint route matches to avoid repeated regex matching (webserver.hpp, webserver.cpp) - Optimize standardize_url to modify in-place instead of creating extra copies (http_utils.cpp)
- Add bounds checking for url_pieces[chunks[i]] to prevent potential buffer overflow when cached endpoint chunks don't match request pieces - Extract URL parameter extraction from cache hit/miss paths into single block after match resolution (DRY fix) - Use const references for vectors instead of by-value copies to avoid heap allocations under cache lock - Add empty-string guard to standardize_url before calling back() - Extract ensure_path_pieces_cached() helper to deduplicate caching logic in get_path_pieces() and get_path_piece() - Add RFC 7230 comment documenting case-sensitive strcmp intent
The matched_ep pointer referenced data inside route_cache_list that could be invalidated by another thread (via invalidate_route_cache or LRU eviction) after the cache mutex was released. Fix by copying url_pars and chunk_positions vectors while holding the cache lock, so parameter extraction afterwards uses owned data. For cache miss path, the registered_resources_mutex shared lock is still held so direct reference is safe, but copy for consistency.
Move route cache invalidation inside registered_resources_mutex lock in unregister_resource() to prevent use-after-free when threads retrieve cached resource pointers during unregistration. Also invalidate route cache on register_resource() for correctness when resources are dynamically registered at runtime. Add threading model documentation to http_request lazy caching.
…l path Performance optimizations for the HTTP request hot path: - LRU route cache (256 entries) for regex endpoint matching - Separate regex endpoint map to avoid scanning all endpoints - Replace istringstream with manual find() loop in string_split - Replace sscanf with direct hex arithmetic in http_unescape - Cache path_pieces in http_request to avoid repeated tokenization - Flatten unique_ptr<string> to plain string in modded_request - Case-sensitive strcmp for HTTP methods per RFC 7230 - Standardize_url optimization with std::unique and pop_back
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #365 +/- ##
==========================================
- Coverage 70.54% 70.49% -0.05%
==========================================
Files 28 28
Lines 1436 1488 +52
Branches 570 593 +23
==========================================
+ Hits 1013 1049 +36
- Misses 35 43 +8
- Partials 388 396 +8
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
Fixes cpplint build/include_what_you_use warning.
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
unregister_resourcecache invalidationTest plan
make checkpasses all 14 testsmake valgrind-checkshows no memory errorsunregister_resourceproperly invalidates cache entries