Skip to content

Conversation

@adam-fowler
Copy link
Collaborator

This means we don't need to add additional support to the channel handler to support RESP2 subscriptions
Also a number of commands return different types based on whether you are using RESP2 or 3. Sticking to one return type would make life a lot easier.

@github-actions
Copy link

github-actions bot commented May 3, 2025

✅ Pull request no significant performance differences ✅

Summary

New baseline 'pull_request' is WITHIN the 'main' baseline thresholds.

Full Benchmark Comparison

Comparing results between 'main' and 'pull_request'

Host '2be58c2ba704' with 2 'x86_64' processors with 7 GB memory, running:
#12~24.04.1-Ubuntu SMP Mon Mar 10 19:00:39 UTC 2025

ValkeyBenchmarks

GET benchmark metrics

Malloc (total): results within specified thresholds, fold down for details.

Malloc (total) * p0 p25 p50 p75 p90 p99 p100 Samples
main 4 4 4 4 12 24 24 11
pull_request 4 4 4 4 12 24 24 11
Δ 0 0 0 0 0 0 0 0
Improvement % 0 0 0 0 0 0 0 0

HashSlot – {user}.whatever metrics

Malloc (total): results within specified thresholds, fold down for details.

Malloc (total) * p0 p25 p50 p75 p90 p99 p100 Samples
main 0 0 0 0 0 0 0 21
pull_request 0 0 0 0 0 0 0 20
Δ 0 0 0 0 0 0 0 -1
Improvement % 0 0 0 0 0 0 0 -1

ValkeyCommandEncoder – Command with 7 words metrics

Malloc (total): results within specified thresholds, fold down for details.

Malloc (total) * p0 p25 p50 p75 p90 p99 p100 Samples
main 0 0 0 0 0 0 0 745
pull_request 0 0 0 0 0 0 0 743
Δ 0 0 0 0 0 0 0 -2
Improvement % 0 0 0 0 0 0 0 -2

ValkeyCommandEncoder – Simple GET metrics

Malloc (total): results within specified thresholds, fold down for details.

Malloc (total) * p0 p25 p50 p75 p90 p99 p100 Samples
main 0 0 0 0 0 0 0 1957
pull_request 0 0 0 0 0 0 0 1951
Δ 0 0 0 0 0 0 0 -6
Improvement % 0 0 0 0 0 0 0 -6

ValkeyCommandEncoder – Simple MGET 15 keys metrics

Malloc (total): results within specified thresholds, fold down for details.

Malloc (total) * p0 p25 p50 p75 p90 p99 p100 Samples
main 0 0 0 0 0 0 0 359
pull_request 0 0 0 0 0 0 0 363
Δ 0 0 0 0 0 0 0 4
Improvement % 0 0 0 0 0 0 0 4

@adam-fowler adam-fowler requested a review from fabianfett May 5, 2025 07:10
Copy link
Collaborator

@fabianfett fabianfett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is mostly about not supporting RESP2 today. We should reconsider this once we reached 1.0.

@adam-fowler adam-fowler merged commit 947ff0c into main May 5, 2025
3 checks passed
@adam-fowler adam-fowler deleted the remove-resp2-option branch May 5, 2025 08:20
adam-fowler added a commit that referenced this pull request Jul 11, 2025
adam-fowler added a commit that referenced this pull request Jul 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants