Merge remote-tracking branch 'origin/topic/timw/storage-redis-expire-string-view'

* origin/topic/timw/storage-redis-expire-string-view:
  Use std::string_view in Redis::DoExpire to avoid copies

(cherry picked from commit 58d71d2fa3)
This commit is contained in:
Tim Wojtulewicz 2025-05-05 10:44:27 -07:00
parent 99acfc6534
commit 4f683ee1f8
3 changed files with 22 additions and 10 deletions

View file

@ -1,3 +1,9 @@
7.2.0-rc1.8 | 2025-05-05 11:21:13 -0700
* Use std::string_view in Redis::DoExpire to avoid copies (Tim Wojtulewicz, Corelight)
(cherry picked from commit 58d71d2fa35007695cd0f6ea3ec975df67c91bab)
7.2.0-rc1.7 | 2025-05-05 11:17:50 -0700 7.2.0-rc1.7 | 2025-05-05 11:17:50 -0700
* QUIC: Extract reset_crypto() function (Arne Welzel, Corelight) * QUIC: Extract reset_crypto() function (Arne Welzel, Corelight)

View file

@ -1 +1 @@
7.2.0-rc1.7 7.2.0-rc1.8

View file

@ -388,27 +388,33 @@ void Redis::DoExpire(double current_network_time) {
return; return;
} }
std::vector<std::string> elements; std::vector<std::string_view> elements;
for ( size_t i = 0; i < reply->elements; i++ ) elements.reserve(reply->elements);
elements.emplace_back(reply->element[i]->str);
freeReplyObject(reply); for ( size_t i = 0; i < reply->elements; i++ )
elements.emplace_back(reply->element[i]->str, reply->element[i]->len);
// TODO: it's possible to pass multiple keys to a DEL operation but it requires // TODO: it's possible to pass multiple keys to a DEL operation but it requires
// building an array of the strings, building up the DEL command with entries, // building an array of the strings, building up the DEL command with entries,
// and passing the array as a block somehow. There's no guarantee it'd be faster // and passing the array as a block somehow. There's no guarantee it'd be faster
// anyways. // anyways.
for ( const auto& e : elements ) { for ( const auto& e : elements ) {
status = redisAsyncCommand(async_ctx, redisGeneric, NULL, "DEL %s:%s", key_prefix.data(), e.c_str()); // redisAsyncCommand usually takes a printf-style string, except the parser used by
// hiredis doesn't handle lengths passed with strings correctly (it hangs indefinitely).
// Use util::fmt here instead it handles it.
status = redisAsyncCommand(async_ctx, redisGeneric, NULL,
util::fmt("DEL %s:%.*s", key_prefix.data(), static_cast<int>(e.size()), e.data()));
++active_ops; ++active_ops;
Poll(); Poll();
redisReply* reply = reply_queue.front(); redisReply* del_reply = reply_queue.front();
reply_queue.pop_front(); reply_queue.pop_front();
freeReplyObject(reply); freeReplyObject(del_reply);
// TODO: do we care if this failed? // TODO: do we care if this failed?
} }
freeReplyObject(reply);
// Remove all of the elements from the range-set that match the time range. // Remove all of the elements from the range-set that match the time range.
redisAsyncCommand(async_ctx, redisGeneric, NULL, "ZREMRANGEBYSCORE %s_expire -inf %f", key_prefix.data(), redisAsyncCommand(async_ctx, redisGeneric, NULL, "ZREMRANGEBYSCORE %s_expire -inf %f", key_prefix.data(),
current_network_time); current_network_time);
@ -416,9 +422,9 @@ void Redis::DoExpire(double current_network_time) {
++active_ops; ++active_ops;
Poll(); Poll();
reply = reply_queue.front(); redisReply* rem_range_reply = reply_queue.front();
reply_queue.pop_front(); reply_queue.pop_front();
freeReplyObject(reply); freeReplyObject(rem_range_reply);
// TODO: do we care if this failed? // TODO: do we care if this failed?
} }