Tracemalloc traces do not include original stack trace length

[jd]

BPO	37961
Nosy	@vstinner, @blueyed, @jd, @miss-islington
PRs	bpo-37961: tracemalloc: store the actual length of traceback #15545 bpo-37961: fix regression in tracemalloc.Traceback.__repr__ #23805 [3.9] bpo-37961: Fix regression in tracemalloc.Traceback.__repr__ (GH-23805) #23809

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2019-10-15.13:10:34.367>
created_at = <Date 2019-08-27.08:23:25.285>
labels = ['interpreter-core', '3.9']
title = 'Tracemalloc traces do not include original stack trace length'
updated_at = <Date 2020-12-16.22:01:23.615>
user = 'https://github.com/jd'

bugs.python.org fields:

activity = <Date 2020-12-16.22:01:23.615>
actor = 'miss-islington'
assignee = 'none'
closed = True
closed_date = <Date 2019-10-15.13:10:34.367>
closer = 'vstinner'
components = ['Interpreter Core']
creation = <Date 2019-08-27.08:23:25.285>
creator = 'jd'
dependencies = []
files = []
hgrepos = []
issue_num = 37961
keywords = ['patch']
message_count = 10.0
messages = ['350616', '350809', '350813', '350865', '350875', '350883', '354717', '354722', '383207', '383212']
nosy_count = 4.0
nosy_names = ['vstinner', 'blueyed', 'jd', 'miss-islington']
pr_nums = ['15545', '23805', '23809']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue37961'
versions = ['Python 3.9']

[jd]

When using the tracemalloc module, the maximum number of frames that are captured is specified at startup via a value to the start method.

However, if the number of frames is truncated, there's no way to know the original length of the stack traces.

[vstinner]

However, if the number of frames is truncated, there's no way to know the original length of the stack traces.

PR 15545 makes each trace 4 bytes (sizeof int) larger. Would it be enough for you to only know if the traceback is truncated?

tracemalloc is already "memory heavy", so I don't think that making trace_t larger is a blocker issue :-)

[jd]

That's a good question. Considering that Py_DEFAULT_RECURSION_LIMIT is 1000, we could probably settle on 2 bytes by using an uint16_t which ought to be enough unless people regularly trace Python stack traces bigger that are bigger than 2^16.

[vstinner]

I'm fine with limiting MAX_NFRAME to USHRT_MAX and change traceback_t.nframe type to unsigned short (16 bits). Storing 65536 should be way enough. It would be great if you manage to add your new field without making traceback_t larger.

In fact, I didn't check: maybe traceback_t size is not change by your PR, because of memory aligment and padding.

By the way, first I taught that your modified trace_t: no, you modified traceback_t. _tracemalloc ensures that a traceback_t instance is allocated exactly once in the heap memory, to reduce the memory footprint. So making traceback_t larger should impact less the memory than making trace_t larger.

[jd]

In fact, I didn't check: maybe traceback_t size is not change by your PR, because of memory aligment and padding.

It is changed. The current size is should be determined by sizeof(Py_uhash_t + int + int) which is 4 + 4 + 4, so 12 aligned/padded.

Using a short will add 2 bytes and potentially some space for 2 more bytes in the future. :)

[vstinner]

With the following change, traceback_t size changes from 24 bytes to 32 bytes:

diff --git a/Modules/_tracemalloc.c b/Modules/_tracemalloc.c
index ee32ac29b7..8a820db907 100644
--- a/Modules/_tracemalloc.c
+++ b/Modules/_tracemalloc.c
@@ -79,6 +79,7 @@ __attribute__((packed))
 typedef struct {
     Py_uhash_t hash;
     int nframe;
+    int length;
     frame_t frames[1];
 } traceback_t;
 
@@ -1640,6 +1641,8 @@ PyInit__tracemalloc(void)
     if (tracemalloc_init() < 0)
         return NULL;
 
+    printf("sizeof(traceback_t) = %zu bytes\n", sizeof(traceback_t));
+
     return m;
 }

The following structure size is 24 bytes, so no change:

typedef struct {
    Py_uhash_t hash;
    short nframe;
    short length;
    frame_t frames[1];
} traceback_t;

The short approach is promising :-)

[vstinner]

New changeset 8d59eb1 by Victor Stinner (Julien Danjou) in branch 'master':
bpo-37961, tracemalloc: add Traceback.total_nframe (GH-15545)
8d59eb1

[vstinner]

Thanks for your contribution Julien, I merged your PR.

[vstinner]

New changeset 051b981 by Daniel Hahler in branch 'master':
bpo-37961: Fix regression in tracemalloc.Traceback.__repr__ (GH-23805)
051b981

[miss-islington]

New changeset 78062e0 by Miss Islington (bot) in branch '3.9':
bpo-37961: Fix regression in tracemalloc.Traceback.__repr__ (GH-23805)
78062e0