Skip to content

Conversation

adam-fowler
Copy link
Collaborator

@adam-fowler adam-fowler commented Sep 25, 2025

See #225

I made an attempt at this a while back but it was decided not to merge as the error type didn't contain enough information. Preferably it would contain a stack of labels to show what you were decoding. We still don't have that but this time the error is extensible such that we can add it later if needed.

Signed-off-by: Adam Fowler <adamfowler71@gmail.com>
Signed-off-by: Adam Fowler <adamfowler71@gmail.com>
Signed-off-by: Adam Fowler <adamfowler71@gmail.com>
@adam-fowler adam-fowler marked this pull request as draft September 25, 2025 19:55
Copy link

github-actions bot commented Sep 25, 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 '50fa63915da9' with 4 'x86_64' processors with 15 GB memory, running:
#18~24.04.1-Ubuntu SMP Sat Jun 28 04:46:03 UTC 2025

ValkeyBenchmarks

Client: GET benchmark metrics

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

Malloc (total) * p0 p25 p50 p75 p90 p99 p100 Samples
main 82 83 85 86 87 87 87 6
pull_request 83 83 84 88 89 89 89 6
Δ 1 0 -1 2 2 2 2 0
Improvement % -1 0 1 -2 -2 -2 -2 0

Client: GET benchmark | parallel 20 | 20 concurrent connections metrics

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

Malloc (total) * p0 p25 p50 p75 p90 p99 p100 Samples
main 96 101 103 107 110 114 114 25
pull_request 92 101 103 106 109 113 113 25
Δ -4 0 0 -1 -1 -1 -1 0
Improvement % 4 0 0 1 1 1 1 0

Connection: 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 4 4 4 9
pull_request 4 4 4 4 4 4 4 8
Δ 0 0 0 0 0 0 0 -1
Improvement % 0 0 0 0 0 0 0 -1

Connection: GET benchmark – NoOpTracer metrics

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

Malloc (total) * p0 p25 p50 p75 p90 p99 p100 Samples
main 7 8 8 10 10 10 10 8
pull_request 7 8 8 10 10 10 10 8
Δ 0 0 0 0 0 0 0 0
Improvement % 0 0 0 0 0 0 0 0

Connection: Pipeline array benchmark metrics

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

Malloc (total) * p0 p25 p50 p75 p90 p99 p100 Samples
main 33 33 33 34 34 34 34 6
pull_request 33 33 34 34 34 34 34 6
Δ 0 0 1 0 0 0 0 0
Improvement % 0 0 -3 0 0 0 0 0

Connection: Pipeline benchmark metrics

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

Malloc (total) * p0 p25 p50 p75 p90 p99 p100 Samples
main 33 33 33 34 34 34 34 6
pull_request 33 33 33 34 34 34 34 6
Δ 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 18
pull_request 0 0 0 0 0 0 0 18
Δ 0 0 0 0 0 0 0 0
Improvement % 0 0 0 0 0 0 0 0

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 743
pull_request 0 0 0 0 0 0 0 728
Δ 0 0 0 0 0 0 0 -15
Improvement % 0 0 0 0 0 0 0 -15

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 1897
pull_request 0 0 0 0 0 0 0 1856
Δ 0 0 0 0 0 0 0 -41
Improvement % 0 0 0 0 0 0 0 -41

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 362
pull_request 0 0 0 0 0 0 0 354
Δ 0 0 0 0 0 0 0 -8
Improvement % 0 0 0 0 0 0 0 -8

@adam-fowler adam-fowler marked this pull request as ready for review October 8, 2025 15:39
Copy link
Collaborator

@nilanshu-sharma nilanshu-sharma left a comment

Choose a reason for hiding this comment

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

Looks good, just one minor comment around handling empty arrays.

}
/// Does not match the expected array size
public static func invalidArraySize(_ array: RESPToken.Array) -> Self {
.init(.invalidArraySize, token: .array(array))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could be worth adding a message just like tokenMismatch() to make this error more descriptive. For example for empty array, the message can say "Empty Array" and so on.

Signed-off-by: Adam Fowler <adamfowler71@gmail.com>
Signed-off-by: Adam Fowler <adamfowler71@gmail.com>
Signed-off-by: Adam Fowler <adamfowler71@gmail.com>
@adam-fowler adam-fowler merged commit ac45bb2 into main Oct 11, 2025
13 checks passed
@adam-fowler adam-fowler deleted the decode-error branch October 11, 2025 07:21
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.

2 participants