Skip to content

Commit c300608

Browse files
LeeCampbellclaude
andauthored
fix(#99): LongHistogram round-trip fails with significantDigits != 3 (#130)
* test(#99): add tests reproducing partial-read bug in ByteBuffer.ReadFrom ByteBufferTests uses a PartialReadStream wrapper to deterministically prove that ReadFrom only returns the first chunk when the underlying stream delivers data in multiple reads — exactly the behaviour of DeflateStream at DEFLATE block boundaries. Also adds parameterised round-trip tests varying significantDigits (1-5) and value counts (1,000–50,000) across the compressed encode/decode and full log-writer/reader pipelines. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(#99): LongHistogram round-trip fails with significantDigits != 3 Two bugs in the DEFLATE compression/decompression path: 1. ByteBuffer.ReadFrom called Stream.Read() once, but DeflateStream may return fewer bytes than requested at compression-block boundaries. Replaced with a loop that reads until all bytes are consumed or end-of-stream. 2. ByteBufferExtensions.CompressedCopy compressed the entire buffer capacity (source.ToArray()) instead of only the bytes written (source.Position). For significantDigits=4 the intermediate buffer is ~2.75 MB but only ~30-40 KB is data; the trailing zeros inflated the compressed stream and increased the chance of partial reads on decompression. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 752ffa0 commit c300608

5 files changed

Lines changed: 130 additions & 2 deletions

File tree

HdrHistogram.UnitTests/HistogramEncodingTestBase.cs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,25 @@ public void Given_a_populated_Histogram_When_encoded_and_decoded_with_compressio
3030
HistogramAssert.AreValueEqual(source, result);
3131
}
3232

33+
[Theory]
34+
[InlineData(1, 10000)]
35+
[InlineData(2, 10000)]
36+
[InlineData(3, 10000)]
37+
[InlineData(4, 10000)]
38+
[InlineData(5, 10000)]
39+
[InlineData(4, 50000)]
40+
public void Given_varying_significant_digits_When_compressed_encoded_and_decoded_Then_data_is_preserved(int significantDigits, int valueCount)
41+
{
42+
var highestTrackableValue = 3600L * 1000 * 1000 * 1000;
43+
var source = Create(highestTrackableValue, significantDigits);
44+
for (long i = 0; i < valueCount; i++)
45+
{
46+
source.RecordValue(i * 1000);
47+
}
48+
var result = CompressedEncodeDecode(source);
49+
HistogramAssert.AreValueEqual(source, result);
50+
}
51+
3352
[Fact]
3453
public void Given_a_Histogram_populated_with_full_range_of_values_When_encoded_and_decoded_Then_data_is_preserved()
3554
{

HdrHistogram.UnitTests/Persistence/HistogramLogReaderWriterTestBase.cs

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,36 @@ public void CanRoundTripSingleHistogram(long highestTrackableValue, int signific
5353
HistogramAssert.AreValueEqual(histogram, actualHistograms.Single());
5454
}
5555

56+
[Theory]
57+
[InlineData(1, 1000)]
58+
[InlineData(2, 1000)]
59+
[InlineData(3, 1000)]
60+
[InlineData(4, 1000)]
61+
[InlineData(5, 1000)]
62+
[InlineData(1, 10000)]
63+
[InlineData(2, 10000)]
64+
[InlineData(3, 10000)]
65+
[InlineData(4, 10000)]
66+
[InlineData(5, 10000)]
67+
[InlineData(4, 50000)]
68+
[InlineData(5, 50000)]
69+
public void CanRoundTripSingleHistogramWithVaryingSignificantDigits(int significantDigits, int valueCount)
70+
{
71+
var highestTrackableValue = OneHourOfNanoseconds;
72+
var histogram = Create(highestTrackableValue, significantDigits);
73+
for (long i = 0; i < valueCount; i++)
74+
{
75+
histogram.RecordValue(i * 1000);
76+
}
77+
78+
histogram.SetTimes();
79+
var data = histogram.WriteLog();
80+
var actualHistograms = data.ReadHistograms();
81+
82+
Assert.Single(actualHistograms);
83+
HistogramAssert.AreValueEqual(histogram, actualHistograms.Single());
84+
}
85+
5686
protected void RoundTripSingleHistogramsWithFullRangesOfCountsAndValues(long count)
5787
{
5888
var value = 1;
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
using System;
2+
using System.IO;
3+
using HdrHistogram.Utilities;
4+
using Xunit;
5+
6+
namespace HdrHistogram.UnitTests.Utilities
7+
{
8+
public class ByteBufferTests
9+
{
10+
/// <summary>
11+
/// Reproduces Issue #99.
12+
/// <see cref="ByteBuffer.ReadFrom"/> must loop until all requested bytes
13+
/// have been read, because <see cref="Stream.Read"/> is permitted to
14+
/// return fewer bytes than requested — even when more data is available.
15+
/// <see cref="System.IO.Compression.DeflateStream"/> does exactly this
16+
/// at DEFLATE block boundaries.
17+
/// </summary>
18+
[Theory]
19+
[InlineData(1024, 100)]
20+
[InlineData(4096, 511)]
21+
[InlineData(8192, 1000)]
22+
public void ReadFrom_returns_all_bytes_when_stream_returns_partial_reads(
23+
int totalBytes, int maxBytesPerRead)
24+
{
25+
var data = new byte[totalBytes];
26+
new Random(42).NextBytes(data);
27+
var stream = new PartialReadStream(new MemoryStream(data), maxBytesPerRead);
28+
var buffer = ByteBuffer.Allocate(totalBytes);
29+
30+
int bytesRead = buffer.ReadFrom(stream, totalBytes);
31+
32+
Assert.Equal(totalBytes, bytesRead);
33+
}
34+
35+
/// <summary>
36+
/// A stream wrapper that returns at most <c>maxBytesPerRead</c> bytes
37+
/// per <see cref="Read"/> call, simulating the behaviour of
38+
/// <see cref="System.IO.Compression.DeflateStream"/> at compression
39+
/// block boundaries.
40+
/// </summary>
41+
private sealed class PartialReadStream : Stream
42+
{
43+
private readonly Stream _inner;
44+
private readonly int _maxBytesPerRead;
45+
46+
public PartialReadStream(Stream inner, int maxBytesPerRead)
47+
{
48+
_inner = inner;
49+
_maxBytesPerRead = maxBytesPerRead;
50+
}
51+
52+
public override int Read(byte[] buffer, int offset, int count)
53+
{
54+
return _inner.Read(buffer, offset, Math.Min(count, _maxBytesPerRead));
55+
}
56+
57+
public override bool CanRead => true;
58+
public override bool CanSeek => false;
59+
public override bool CanWrite => false;
60+
public override long Length => throw new NotSupportedException();
61+
public override long Position
62+
{
63+
get => throw new NotSupportedException();
64+
set => throw new NotSupportedException();
65+
}
66+
public override void Flush() { }
67+
public override long Seek(long offset, SeekOrigin origin) => throw new NotSupportedException();
68+
public override void SetLength(long value) => throw new NotSupportedException();
69+
public override void Write(byte[] buffer, int offset, int count) => throw new NotSupportedException();
70+
}
71+
}
72+
}

HdrHistogram/Utilities/ByteBuffer.cs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,14 @@ public int Remaining()
8383
/// <returns>The number of bytes read.</returns>
8484
public int ReadFrom(System.IO.Stream source, int length)
8585
{
86-
return source.Read(_internalBuffer, Position, length);
86+
int totalRead = 0;
87+
while (totalRead < length)
88+
{
89+
int bytesRead = source.Read(_internalBuffer, Position + totalRead, length - totalRead);
90+
if (bytesRead == 0) break;
91+
totalRead += bytesRead;
92+
}
93+
return totalRead;
8794
}
8895

8996
/// <summary>

HdrHistogram/Utilities/ByteBufferExtensions.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ internal static class ByteBufferExtensions
1515
public static int CompressedCopy(this ByteBuffer target, ByteBuffer source, int targetOffset)
1616
{
1717
byte[] compressed;
18-
using (var ms = new MemoryStream(source.ToArray()))
18+
using (var ms = new MemoryStream(source.ToArray(), 0, source.Position))
1919
{
2020
compressed = Compress(ms);
2121
}

0 commit comments

Comments
 (0)