SolidWallOfCode commented on code in PR #10940:
URL: https://github.com/apache/trafficserver/pull/10940#discussion_r1430486525
##########
src/proxy/http/HttpSM.cc:
##########
@@ -5721,18 +5721,14 @@ HttpSM::mark_host_failure(ResolveInfo *info, ts_time
time_down)
if (++info->active->fail_count >=
t_state.txn_conf->connect_attempts_rr_retries) {
if (info->active) {
if (info->active->last_failure.load() == TS_TIME_ZERO) {
- char *url_str =
t_state.hdr_info.client_request.url_string_get(&t_state.arena, nullptr);
+ char *url_str =
t_state.hdr_info.client_request.url_string_get_ref(nullptr);
Review Comment:
The copy on the heap is a cache and it's not duplicated if already present.
If it's already there, `url_string_get_ref` will return that existing copy.
Therefore in the worst case it's the same, and the best case it's very fast
because the URL doesn't have to be printed. The original code used `Arena` as
temporary space, but as noted that doesn't avoid the copy to the heap.
This isn't really done as I'd like, but changing things to do the better
style is a very heavy lift (e.g., this should be returned `TextView` not `char
const *`).
`MemArena` already supports the[ use of static blocks for memory
allocation](http://docs.solidwallofcode.com/libswoc/code/MemArena.en.html#static-memory).
Speaking of which, that would be good in another PR I'm working on for which
this is a precursor.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
