Skip to content

Commit b2b10db

Browse files
fixup! Store Tachyon samples in bounded columnar blocks
The stack cache hashed stacks with Py_HashBuffer(), which depends on the per-process hash secret. Since the hash picks which stacks stay cached for REUSE, the same samples produced a different file in each run.
1 parent 0d61331 commit b2b10db

3 files changed

Lines changed: 45 additions & 4 deletions

File tree

‎Lib/test/test_profiling/test_sampling_profiler/test_binary_format.py‎

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,17 @@ def export(self, filename):
127127
pass
128128

129129

130+
def stack_cache_hash(frame_ids):
131+
"""Hash a stack the way the writer's stack cache does."""
132+
mask = (1 << 64) - 1
133+
value = 0xcbf29ce484222325
134+
for frame_id in frame_ids:
135+
value = ((value ^ frame_id) * 0x100000001b3) & mask
136+
value ^= value >> 33
137+
value = (value * 0xff51afd7ed558ccd) & mask
138+
return value ^ (value >> 33)
139+
140+
130141
def encode_varint(value):
131142
"""Encode an unsigned varint."""
132143
out = bytearray()
@@ -1046,7 +1057,7 @@ def test_writer_reuses_colliding_stacks(self):
10461057
"""Stacks sharing a cache bucket are both stored as STACK_REUSE."""
10471058
buckets = {}
10481059
for frame_id in range(2, 4099):
1049-
bucket = hash(struct.pack("=3I", 0, 1, frame_id)) % 4096
1060+
bucket = stack_cache_hash((0, 1, frame_id)) % 4096
10501061
if bucket in buckets:
10511062
first_id = buckets[bucket]
10521063
second_id = frame_id
@@ -1927,6 +1938,19 @@ def test_replay_reads_constant_stream_and_residual_columns(self):
19271938
self.assertEqual(timestamps,
19281939
[2 + i * (interval + 1) for i in range(3)])
19291940

1941+
def test_replay_rejects_unterminated_constant_varint(self):
1942+
"""A constant byte that never ends a varint is rejected."""
1943+
for column, part in ((self.COLUMN_STREAM_IDS, "stream_ids"),
1944+
(self.COLUMN_TIMESTAMP_RESIDUALS, "residuals")):
1945+
with self.subTest(part):
1946+
filename = self._write_single_block(
1947+
**{part: b"\x80"},
1948+
descriptors={column: (self.COLUMN_CONSTANT, 1)})
1949+
with BinaryReader(filename) as reader:
1950+
with self.assertRaisesRegex(ValueError,
1951+
"incomplete varint"):
1952+
reader.replay_samples(RawCollector())
1953+
19301954
def test_replay_rejects_reentrant_replay(self):
19311955
"""Replaying from inside the same reader's collector raises."""
19321956
filename = self._write_single_block(
@@ -1986,8 +2010,16 @@ def test_replay_rejects_invalid_block(self):
19862010
(self.COLUMN_CONSTANT, 2)})),
19872011
("stream index column has trailing data",
19882012
dict(num_samples=2, header_samples=1)),
2013+
# A constant column longer than the sample count is expanded
2014+
# and then rejected like any other trailing data
2015+
("stream index column has trailing data",
2016+
dict(descriptors={self.COLUMN_STREAM_IDS:
2017+
(self.COLUMN_CONSTANT, 2)})),
19892018
("timestamp column has trailing data",
19902019
dict(residuals=b"\x00\x00")),
2020+
("timestamp column has trailing data",
2021+
dict(descriptors={self.COLUMN_TIMESTAMP_RESIDUALS:
2022+
(self.COLUMN_CONSTANT, 2)})),
19912023
("frame index out of range",
19922024
dict(instructions=b"\x01\x00\x01", frame_ids=b"\x05")),
19932025
# Frame IDs 65536 and 2**24 only have a byte in the third or

‎Modules/_remote_debugging/binary_io.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -294,10 +294,11 @@ typedef struct {
294294
uint64_t thread_id;
295295
uint32_t interpreter_id;
296296

297-
/* Stream index in the current block */
297+
/* Stream index in the current block, or UINT32_MAX */
298298
uint32_t block_stream_index;
299299
} ThreadEntry;
300300

301+
/* Delta encoding state of a stream, reset with each block */
301302
typedef struct {
302303
uint64_t prev_timestamp;
303304
size_t thread_index;

‎Modules/_remote_debugging/binary_io_writer.c‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -765,11 +765,19 @@ block_flush(BinaryWriter *writer)
765765
return 0;
766766
}
767767

768+
/* FNV-1a over the frame indices, the same in every process */
768769
static Py_uhash_t
769770
stack_hash(const uint32_t *stack, size_t depth)
770771
{
771-
return (Py_uhash_t)Py_HashBuffer(
772-
stack, (Py_ssize_t)(depth * sizeof(*stack)));
772+
uint64_t hash = 0xcbf29ce484222325;
773+
for (size_t i = 0; i < depth; i++) {
774+
hash = (hash ^ stack[i]) * 0x100000001b3;
775+
}
776+
/* Mix the high bits into the low bits that select the cache bucket */
777+
hash ^= hash >> 33;
778+
hash *= 0xff51afd7ed558ccd;
779+
hash ^= hash >> 33;
780+
return (Py_uhash_t)hash;
773781
}
774782

775783
static size_t

0 commit comments

Comments
 (0)