Skip to content

Commit 470c558

Browse files
ivonastojanovicivonastojanovic
committed
fixup! Store Tachyon samples in bounded columnar blocks
Replay currently builds a new Python FrameInfo object every time a frame appears in a stack. These objects are immutable, so keep a small, bounded cache on the reader and reuse them while still creating a new frame list for each stack.
1 parent 442fc17 commit 470c558

3 files changed

Lines changed: 120 additions & 54 deletions

File tree

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

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1192,6 +1192,19 @@ def test_progress_updates_across_blocks(self):
11921192

11931193
def test_reader_replays_twice(self):
11941194
"""Replaying twice gives the same samples and statistics."""
1195+
class RetainingCollector(RawCollector):
1196+
def __init__(self):
1197+
super().__init__()
1198+
self.frame_lists = []
1199+
1200+
def collect(self, stack_frames, timestamps_us):
1201+
self.frame_lists.extend(
1202+
thread.frame_info
1203+
for interp in stack_frames
1204+
for thread in interp.threads
1205+
)
1206+
super().collect(stack_frames, timestamps_us)
1207+
11951208
samples = [
11961209
[make_interpreter(0, [
11971210
make_thread(1, [make_frame("a.py", i % 3, "f")]),
@@ -1201,12 +1214,21 @@ def test_reader_replays_twice(self):
12011214
]
12021215
filename = self.create_binary_file(samples)
12031216
with BinaryReader(filename) as reader:
1204-
first = RawCollector()
1205-
second = RawCollector()
1217+
first = RetainingCollector()
1218+
second = RetainingCollector()
12061219
self.assertEqual(reader.replay_samples(first), 20)
12071220
self.assertEqual(reader.replay_samples(second), 20)
12081221
self.assertEqual(reader.get_stats()["total_samples"], 20)
12091222
self.assertEqual(first.by_thread, second.by_thread)
1223+
for first_frames, second_frames in zip(
1224+
first.frame_lists, second.frame_lists, strict=True
1225+
):
1226+
self.assertIsNot(first_frames, second_frames)
1227+
self.assertEqual(len(first_frames), len(second_frames))
1228+
for first_frame, second_frame in zip(
1229+
first_frames, second_frames, strict=True
1230+
):
1231+
self.assertIs(first_frame, second_frame)
12101232

12111233
def test_writer_repeat_run_spans_interleaved_streams(self):
12121234
"""A thread's repeat run continues across other threads' samples."""

‎Modules/_remote_debugging/binary_io.h‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -418,6 +418,11 @@ typedef struct {
418418
uint32_t block_stream_index;
419419
} ReaderThreadState;
420420

421+
typedef struct {
422+
uint32_t frame_index;
423+
PyObject *frame_info;
424+
} ReaderFrameCacheEntry;
425+
421426
/* Decoded sample block */
422427
typedef struct {
423428
BlockHeader header;
@@ -489,6 +494,8 @@ typedef struct {
489494
/* Parsed frame table: array of FrameEntry structures */
490495
FrameEntry *frames;
491496
uint32_t frames_count;
497+
ReaderFrameCacheEntry *frame_info_cache;
498+
size_t frame_info_cache_size;
492499

493500
/* Sample data region */
494501
uint8_t *sample_data;

‎Modules/_remote_debugging/binary_io_reader.c‎

Lines changed: 89 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@
3333
/* Cap per-batch RLE samples to bound the timestamp list (gh-151378) */
3434
#define MAX_RLE_BATCH_SAMPLES 8192
3535

36+
#define FRAME_INFO_CACHE_SIZE 1024
37+
3638
/* ============================================================================
3739
* BINARY READER IMPLEMENTATION
3840
* ============================================================================ */
@@ -699,71 +701,99 @@ reader_get_or_create_thread_state(BinaryReader *reader, uint64_t thread_id,
699701
return ts;
700702
}
701703

702-
/* Build a Python list of FrameInfo objects from frame indices */
703704
static PyObject *
704-
build_frame_list(RemoteDebuggingState *state, BinaryReader *reader,
705-
const uint32_t *frame_indices, size_t stack_depth)
705+
reader_get_frame_info(RemoteDebuggingState *state, BinaryReader *reader,
706+
uint32_t frame_index)
706707
{
707-
PyObject *frame_list = PyList_New(stack_depth);
708-
if (!frame_list) {
708+
if (frame_index >= reader->frames_count) {
709+
PyErr_Format(PyExc_ValueError, "Invalid frame index: %u", frame_index);
709710
return NULL;
710711
}
711-
712-
for (size_t k = 0; k < stack_depth; k++) {
713-
uint32_t frame_idx = frame_indices[k];
714-
if (frame_idx >= reader->frames_count) {
715-
PyErr_Format(PyExc_ValueError, "Invalid frame index: %u", frame_idx);
716-
goto error;
712+
if (reader->frame_info_cache == NULL) {
713+
size_t cache_size = Py_MIN(reader->frames_count,
714+
FRAME_INFO_CACHE_SIZE);
715+
reader->frame_info_cache = PyMem_Calloc(
716+
cache_size, sizeof(*reader->frame_info_cache));
717+
if (reader->frame_info_cache == NULL) {
718+
PyErr_NoMemory();
719+
return NULL;
717720
}
721+
reader->frame_info_cache_size = cache_size;
722+
}
723+
ReaderFrameCacheEntry *entry = &reader->frame_info_cache[
724+
frame_index % reader->frame_info_cache_size];
725+
if (entry->frame_info != NULL && entry->frame_index == frame_index) {
726+
return Py_NewRef(entry->frame_info);
727+
}
718728

719-
FrameEntry *frame = &reader->frames[frame_idx];
729+
FrameEntry *frame = &reader->frames[frame_index];
730+
if (frame->filename_idx >= reader->strings_count ||
731+
frame->funcname_idx >= reader->strings_count) {
732+
PyErr_SetString(PyExc_ValueError, "Invalid string index in frame");
733+
return NULL;
734+
}
720735

721-
if (frame->filename_idx >= reader->strings_count ||
722-
frame->funcname_idx >= reader->strings_count) {
723-
PyErr_SetString(PyExc_ValueError, "Invalid string index in frame");
724-
goto error;
725-
}
736+
PyObject *frame_info = PyStructSequence_New(state->FrameInfo_Type);
737+
if (!frame_info) {
738+
return NULL;
739+
}
726740

727-
PyObject *frame_info = PyStructSequence_New(state->FrameInfo_Type);
728-
if (!frame_info) {
729-
goto error;
741+
/* Build location tuple with full position info */
742+
PyObject *location;
743+
if (frame->lineno != LOCATION_NOT_AVAILABLE) {
744+
location = Py_BuildValue("(iiii)",
745+
frame->lineno,
746+
frame->end_lineno,
747+
frame->column,
748+
frame->end_column);
749+
if (!location) {
750+
Py_DECREF(frame_info);
751+
return NULL;
730752
}
753+
}
754+
else {
755+
location = Py_NewRef(Py_None);
756+
}
731757

732-
/* Build location tuple with full position info */
733-
PyObject *location;
734-
if (frame->lineno != LOCATION_NOT_AVAILABLE) {
735-
location = Py_BuildValue("(iiii)",
736-
frame->lineno,
737-
frame->end_lineno,
738-
frame->column,
739-
frame->end_column);
740-
if (!location) {
741-
Py_DECREF(frame_info);
742-
goto error;
743-
}
744-
}
745-
else {
746-
location = Py_NewRef(Py_None);
758+
/* Build opcode object */
759+
PyObject *opcode_obj;
760+
if (frame->opcode != OPCODE_NONE) {
761+
opcode_obj = PyLong_FromLong(frame->opcode);
762+
if (!opcode_obj) {
763+
Py_DECREF(location);
764+
Py_DECREF(frame_info);
765+
return NULL;
747766
}
767+
}
768+
else {
769+
opcode_obj = Py_NewRef(Py_None);
770+
}
748771

749-
/* Build opcode object */
750-
PyObject *opcode_obj;
751-
if (frame->opcode != OPCODE_NONE) {
752-
opcode_obj = PyLong_FromLong(frame->opcode);
753-
if (!opcode_obj) {
754-
Py_DECREF(location);
755-
Py_DECREF(frame_info);
756-
goto error;
757-
}
758-
}
759-
else {
760-
opcode_obj = Py_NewRef(Py_None);
761-
}
772+
PyStructSequence_SetItem(frame_info, 0, Py_NewRef(reader->strings[frame->filename_idx]));
773+
PyStructSequence_SetItem(frame_info, 1, location);
774+
PyStructSequence_SetItem(frame_info, 2, Py_NewRef(reader->strings[frame->funcname_idx]));
775+
PyStructSequence_SetItem(frame_info, 3, opcode_obj);
776+
Py_XSETREF(entry->frame_info, Py_NewRef(frame_info));
777+
entry->frame_index = frame_index;
778+
return frame_info;
779+
}
780+
781+
/* Build a Python list of FrameInfo objects from frame indices */
782+
static PyObject *
783+
build_frame_list(RemoteDebuggingState *state, BinaryReader *reader,
784+
const uint32_t *frame_indices, size_t stack_depth)
785+
{
786+
PyObject *frame_list = PyList_New(stack_depth);
787+
if (!frame_list) {
788+
return NULL;
789+
}
762790

763-
PyStructSequence_SetItem(frame_info, 0, Py_NewRef(reader->strings[frame->filename_idx]));
764-
PyStructSequence_SetItem(frame_info, 1, location);
765-
PyStructSequence_SetItem(frame_info, 2, Py_NewRef(reader->strings[frame->funcname_idx]));
766-
PyStructSequence_SetItem(frame_info, 3, opcode_obj);
791+
for (size_t k = 0; k < stack_depth; k++) {
792+
PyObject *frame_info = reader_get_frame_info(
793+
state, reader, frame_indices[k]);
794+
if (frame_info == NULL) {
795+
goto error;
796+
}
767797
PyList_SET_ITEM(frame_list, k, frame_info);
768798
}
769799

@@ -1789,6 +1819,13 @@ binary_reader_close(BinaryReader *reader)
17891819
PyMem_Free(reader->block_streams);
17901820
PyMem_Free(reader->block_stacks);
17911821

1822+
if (reader->frame_info_cache != NULL) {
1823+
for (size_t i = 0; i < reader->frame_info_cache_size; i++) {
1824+
Py_XDECREF(reader->frame_info_cache[i].frame_info);
1825+
}
1826+
PyMem_Free(reader->frame_info_cache);
1827+
}
1828+
17921829
if (reader->strings) {
17931830
for (uint32_t i = 0; i < reader->strings_count; i++) {
17941831
Py_XDECREF(reader->strings[i]);

0 commit comments

Comments
 (0)