classification
Title: Tracemalloc traces do not include original stack trace length
Type: Stage: resolved
Components: Interpreter Core Versions: Python 3.9
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: jd, vstinner
Priority: normal Keywords: patch

Created on 2019-08-27 08:23 by jd, last changed 2019-10-15 13:10 by vstinner. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 15545 merged jd, 2019-08-27 08:26
Messages (8)
msg350616 - (view) Author: Julien Danjou (jd) * Date: 2019-08-27 08:23
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.
msg350809 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-08-29 17:41
> 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 :-)
msg350813 - (view) Author: Julien Danjou (jd) * Date: 2019-08-29 18:48
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.
msg350865 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-08-30 10:48
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.
msg350875 - (view) Author: Julien Danjou (jd) * Date: 2019-08-30 12:46
> 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. :)
msg350883 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-08-30 14:53
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 :-)
msg354717 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-10-15 12:00
New changeset 8d59eb1b66c51b2b918da9881c57d07d08df43b7 by Victor Stinner (Julien Danjou) in branch 'master':
bpo-37961, tracemalloc: add Traceback.total_nframe (GH-15545)
https://github.com/python/cpython/commit/8d59eb1b66c51b2b918da9881c57d07d08df43b7
msg354722 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-10-15 13:10
Thanks for your contribution Julien, I merged your PR.
History
Date User Action Args
2019-10-15 13:10:34vstinnersetstatus: open -> closed
resolution: fixed
messages: + msg354722

stage: patch review -> resolved
2019-10-15 12:00:21vstinnersetmessages: + msg354717
2019-08-30 14:53:53vstinnersetmessages: + msg350883
2019-08-30 12:46:09jdsetmessages: + msg350875
2019-08-30 10:48:29vstinnersetmessages: + msg350865
2019-08-29 18:48:26jdsetmessages: + msg350813
2019-08-29 17:41:45vstinnersetmessages: + msg350809
2019-08-27 08:26:19jdsetkeywords: + patch
stage: patch review
pull_requests: + pull_request15222
2019-08-27 08:25:51xtreaksetnosy: + vstinner
2019-08-27 08:23:25jdcreate