Skip to content

Commit 5db16aa

Browse files
Fix Gecko timestamps when replaying binary profiles
The block reader buffered the first sample for every reconstructed stack. A later stream could therefore be delivered before the earliest sample, causing Gecko to use the wrong replay time origin and produce negative timestamps. Emit newly reconstructed stacks immediately and reserve batching for repeated stacks. This restores the previous replay behavior while keeping repeat batches efficient.
1 parent 874f7d5 commit 5db16aa

2 files changed

Lines changed: 42 additions & 9 deletions

File tree

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

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -475,6 +475,29 @@ def test_binary_replay_preserves_main_thread_for_gecko(self):
475475
self.assertTrue(threads[1]["isMainThread"])
476476
self.assertFalse(threads[2]["isMainThread"])
477477

478+
def test_binary_replay_starts_gecko_at_earliest_sample(self):
479+
"""Binary replay uses the earliest sample as Gecko's time origin."""
480+
filename = self.temp_filename()
481+
collector = BinaryCollector(filename, 1000, compression="none")
482+
for thread_id, name, timestamp in [(1, "first", 1000),
483+
(2, "second", 2000),
484+
(2, "changed", 3000)]:
485+
frame = make_frame(f"{name}.py", 10, name)
486+
collector.collect(
487+
[make_interpreter(0, [make_thread(thread_id, [frame])])],
488+
timestamp_us=timestamp,
489+
)
490+
collector.export(None)
491+
492+
gecko = GeckoCollector(1000)
493+
with BinaryReader(filename) as reader:
494+
self.assertEqual(reader.replay_samples(gecko), 3)
495+
496+
profile = gecko._build_profile()
497+
threads = {thread["tid"]: thread for thread in profile["threads"]}
498+
self.assertEqual(threads[1]["samples"]["time"], [0.0])
499+
self.assertEqual(threads[2]["samples"]["time"], [1.0, 2.0])
500+
478501
def test_multiple_threads_per_sample(self):
479502
"""Multiple threads in one sample roundtrip exactly."""
480503
threads = [
@@ -2197,18 +2220,18 @@ def export(self, filename):
21972220

21982221
def test_rle_long_run_splits_batches(self):
21992222
# Construct one block whose repeat run is longer than a replay batch.
2200-
num_samples = 8193
2223+
num_samples = 8194
22012224
filename = self.create_binary_file([], compression="none")
22022225
data = bytearray(pathlib.Path(filename).read_bytes())
22032226
instructions = (b"\x01\x00\x00" # STACK_REPLACE_SUFFIX 0 0
2204-
+ b"\x00\x80\x40") # STACK_REPEAT 8192
2227+
+ b"\x00\x81\x40") # STACK_REPEAT 8193
22052228
record = (b"TBLK"
2206-
+ b"\x81\x40" # 8193 samples as a varint
2229+
+ b"\x82\x40" # 8194 samples as a varint
22072230
+ b"\x01" # base timestamp
22082231
+ b"\x01\x00\x00" # frame width, status flags
22092232
+ b"\x0c\x00" # stream table: 12 raw bytes
2210-
+ b"\x81\x40\x02" # stream indices: constant
2211-
+ b"\x81\x40\x02" # timestamp residuals: constant
2233+
+ b"\x82\x40\x02" # stream indices: constant
2234+
+ b"\x82\x40\x02" # timestamp residuals: constant
22122235
+ b"\x00\x00" # status bitmaps: empty
22132236
+ bytes([len(instructions), 0]) # raw instructions
22142237
+ b"\x00\x00" # frame IDs: empty
@@ -2232,7 +2255,7 @@ def test_rle_long_run_splits_batches(self):
22322255
count = reader.replay_samples(collector)
22332256
batches = [call.args[1] for call in collector.collect.call_args_list]
22342257
self.assertEqual(count, num_samples)
2235-
self.assertEqual([len(batch) for batch in batches], [8192, 1])
2258+
self.assertEqual([len(batch) for batch in batches], [1, 8192, 1])
22362259
self.assertEqual([ts for batch in batches for ts in batch],
22372260
list(range(1, num_samples + 1)))
22382261

‎Modules/_remote_debugging/binary_io_reader.c‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1422,6 +1422,7 @@ decode_stack_reuse(BinaryReader *reader, DecodedBlock *block,
14221422
return 0;
14231423
}
14241424

1425+
/* Return 1 if a new stack was decoded, 0 if the stack repeats, -1 on error. */
14251426
static int
14261427
decode_stack(BinaryReader *reader, RemoteDebuggingState *state,
14271428
PyObject *collector, DecodedBlock *block,
@@ -1462,7 +1463,7 @@ decode_stack(BinaryReader *reader, RemoteDebuggingState *state,
14621463
}
14631464
stream->has_stack = 1;
14641465
reader->stats.stack_reconstructions++;
1465-
return 0;
1466+
return 1;
14661467
}
14671468

14681469
static int
@@ -1472,7 +1473,8 @@ replay_sample(BinaryReader *reader, RemoteDebuggingState *state,
14721473
ReaderBlockStream *stream =
14731474
&reader->block_streams[block->stream_ids[sample]];
14741475
uint8_t status = block->statuses[sample];
1475-
if (decode_stack(reader, state, collector, block, stream) < 0) {
1476+
int new_stack = decode_stack(reader, state, collector, block, stream);
1477+
if (new_stack < 0) {
14761478
return -1;
14771479
}
14781480

@@ -1495,7 +1497,15 @@ replay_sample(BinaryReader *reader, RemoteDebuggingState *state,
14951497
}
14961498
int result = PyList_Append(stream->batch, timestamp);
14971499
Py_DECREF(timestamp);
1498-
return result;
1500+
if (result < 0) {
1501+
return -1;
1502+
}
1503+
/* Send a new stack's sample immediately so a stream's first sample is
1504+
* not held back behind samples from other streams. */
1505+
if (new_stack) {
1506+
return flush_stream_batch(reader, state, collector, stream);
1507+
}
1508+
return 0;
14991509
}
15001510

15011511
static int

0 commit comments

Comments
 (0)