Skip to content

Conversation

@vstinner
Copy link
Member

@vstinnervstinner commented Oct 23, 2018

tracemalloc now tries to update the traceback when an object is
reused from a "free list" (optimization for faster object creation,
used by the builtin list

Changes:

  • Add _PyTraceMalloc_NewReference() function which tries to update
    the Python traceback of a Python object.
  • _Py_NewReference() now calls _PyTraceMalloc_NewReference().

https://bugs.python.org/issue35053

tracemalloc now tries to update the traceback when an object is reused from a "free list" (optimization for faster object creation, used by the builtin list Changes: * Add _PyTraceMalloc_NewReference() function which tries to update the Python traceback of a Python object. * _Py_NewReference() now calls _PyTraceMalloc_NewReference().
@vstinner
Copy link
MemberAuthor

I modified my PR to make the overhead not significant:

Mean +- std dev: [ref] 20.8 ns +- 0.7 ns -> [patch] 20.5 ns +- 0.3 ns: 1.01x faster (-1%) 

https://bugs.python.org/issue35053#msg328361

"Inline" tracemalloc_add_trace(): Remove one access to the hash table, the trace size doesn't change, so the code can be simplified.
@vstinner
Copy link
MemberAuthor

I also optimized _PyTraceMalloc_NewReference() to make it faster when tracemalloc is tracing memory allocations.

@vstinner
Copy link
MemberAuthor

@methane: Ok, the PR is now waiting for your review :-)

@vstinner
Copy link
MemberAuthor

See also PR #10079 and https://bugs.python.org/issue35059 which convert the _Py_NewReference() macro to a "static inline" function.

@vstinner
Copy link
MemberAuthor

@methane: Thanks for the review!

While reviewing one last time before merging, I found a potential issue: traceback_new() was called when tables were not locked, whereas all other calls to traceback_new() are done with tables locked. I moved the traceback_new() call inside TABLES_LOCK/TABLES_UNLOCK to make the code safer and faster (traceback_new is avoided if it's not needed, if the object is not tracked by tracemalloc).

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@vstinner@methane@the-knights-who-say-ni@bedevere-bot