diff --git a/src/hdr_histogram.c b/src/hdr_histogram.c index 1fe4513..c701361 100644 --- a/src/hdr_histogram.c +++ b/src/hdr_histogram.c @@ -732,12 +732,58 @@ int64_t hdr_min(const struct hdr_histogram* h) static int64_t get_value_from_idx_up_to_count_scalar( const struct hdr_histogram* h, int64_t count_at_percentile) { - int64_t count_to_idx = 0; - for (int32_t idx = 0; idx < h->counts_len; idx++) { - count_to_idx += h->counts[idx]; - if (count_to_idx >= count_at_percentile) + /* Block-summed scan: sum BLK counts, test the running total once per block, + and do the exact per-element walk only for the crossing block. offset != 0 + (decoded/rotated) reads via the offset-aware accessor. */ + enum { BLK = 4 }; + const int64_t* counts = h->counts; + const int32_t n = h->counts_len; + int32_t idx = 0; + int64_t running = 0; + + if (HDR_UNLIKELY(h->normalizing_index_offset != 0)) + { + for (idx = 0; idx < n; idx++) + { + running += counts_get_normalised(h, idx); + if (running >= count_at_percentile) + return hdr_value_at_index(h, idx); + } + return 0; + } + + { + const int32_t blk_limit = n - (n % BLK); + for (; idx < blk_limit; idx += BLK) + { + /* unsigned block sum: cannot overflow under valid state (matches AVX2 path) */ + uint64_t block_sum_u = 0; + int32_t j; + for (j = 0; j < BLK; j++) + block_sum_u += (uint64_t)counts[idx + j]; + if (HDR_UNLIKELY((uint64_t)running + block_sum_u >= (uint64_t)count_at_percentile)) + { + for (j = 0; j < BLK; j++) + { + running += counts[idx + j]; + if (running >= count_at_percentile) + return hdr_value_at_index(h, idx + j); + } + } + else + { + running += (int64_t)block_sum_u; + } + } + } + + for (; idx < n; idx++) + { + running += counts[idx]; + if (running >= count_at_percentile) return hdr_value_at_index(h, idx); } + return 0; } @@ -783,7 +829,8 @@ static int64_t get_value_from_idx_up_to_count(const struct hdr_histogram* h, int { count_at_percentile = count_at_percentile > 0 ? count_at_percentile : 1; #ifdef HDR_HAS_AVX2_DISPATCH - if (__builtin_cpu_supports("avx2")) + /* AVX2 reads counts[] directly; offset != 0 (rotated) must use the scalar scan */ + if (h->normalizing_index_offset == 0 && __builtin_cpu_supports("avx2")) return get_value_from_idx_up_to_count_avx2(h, count_at_percentile); #endif return get_value_from_idx_up_to_count_scalar(h, count_at_percentile); diff --git a/src/hdr_histogram_log.c b/src/hdr_histogram_log.c index 5317c2a..24921c7 100644 --- a/src/hdr_histogram_log.c +++ b/src/hdr_histogram_log.c @@ -554,6 +554,14 @@ static int hdr_decode_compressed_v1( apply_to_counts(h, word_size, counts_array, counts_limit); h->normalizing_index_offset = be32toh(encoding_flyweight.normalizing_index_offset); + /* Reduce a decoded offset into (-counts_len, counts_len): normalize_index applies + only a single +/-counts_len wrap, so an out-of-range offset (untrusted log input) + would index counts[] out of bounds on the offset-aware read paths. No-op for + valid logs, where |offset| < counts_len. */ + if (h->normalizing_index_offset != 0) + { + h->normalizing_index_offset %= h->counts_len; + } h->conversion_ratio = int64_bits_to_double(be64toh(encoding_flyweight.conversion_ratio_bits)); hdr_reset_internal_counters(h); @@ -670,6 +678,14 @@ static int hdr_decode_compressed_v2( } h->normalizing_index_offset = be32toh(encoding_flyweight.normalizing_index_offset); + /* Reduce a decoded offset into (-counts_len, counts_len): normalize_index applies + only a single +/-counts_len wrap, so an out-of-range offset (untrusted log input) + would index counts[] out of bounds on the offset-aware read paths. No-op for + valid logs, where |offset| < counts_len. */ + if (h->normalizing_index_offset != 0) + { + h->normalizing_index_offset %= h->counts_len; + } h->conversion_ratio = int64_bits_to_double(be64toh(encoding_flyweight.conversion_ratio_bits)); hdr_reset_internal_counters(h); diff --git a/test/hdr_histogram_test.c b/test/hdr_histogram_test.c index 0ae65b8..40e3b79 100644 --- a/test/hdr_histogram_test.c +++ b/test/hdr_histogram_test.c @@ -278,6 +278,42 @@ static char* test_percentiles_by_value_at_percentiles(void) return 0; } +/* Singular and plural percentile APIs both scan counts via counts_get_normalised, so + they must locate the same crossing bucket for any normalizing_index_offset. p0 is + excluded: hdr_value_at_percentile special-cases it to lowest_equivalent_value while + the plural API returns highest_equivalent_value, a documented divergence unrelated to + the offset-aware scan. */ +static char* test_percentile_singular_equals_plural_with_offset(void) +{ + struct hdr_histogram* h = NULL; + double percentiles[5] = { 50.0, 90.0, 99.0, 99.9, 100.0 }; + int64_t values[5] = { 0 }; + int i; + + mu_assert("Failed to allocate hdr_histogram", hdr_init(1, INT64_C(3600000000), 3, &h) == 0); + + for (i = 0; i < 10000; i++) + { + hdr_record_value(h, 1000); + } + hdr_record_value(h, 100000000); + + /* Non-zero offset in [1, counts_len): both scans must still agree. */ + h->normalizing_index_offset = h->counts_len / 2; + + mu_assert("value_at_percentiles return should be 0", + hdr_value_at_percentiles(h, percentiles, values, 5) == 0); + + for (i = 0; i < 5; i++) + { + mu_assert("singular != plural under non-zero offset", + hdr_value_at_percentile(h, percentiles[i]) == values[i]); + } + + hdr_close(h); + return 0; +} + static char* test_recorded_values(void) { @@ -741,6 +777,7 @@ static struct mu_result all_tests(void) mu_run_test(test_top_bucket_value_range_no_overflow); mu_run_test(test_percentiles); mu_run_test(test_percentiles_by_value_at_percentiles); + mu_run_test(test_percentile_singular_equals_plural_with_offset); mu_run_test(test_recorded_values); mu_run_test(test_linear_values); mu_run_test(test_logarithmic_values);