From f6464b02abc52e0d3d6ffbe18290a7e7914e9999 Mon Sep 17 00:00:00 2001 From: ichichkine Date: Tue, 4 Oct 2022 19:09:50 -0700 Subject: [PATCH 1/4] Fix _code_replace function --- line_profiler/_line_profiler.pyx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/line_profiler/_line_profiler.pyx b/line_profiler/_line_profiler.pyx index 504a23ca..0491f5eb 100644 --- a/line_profiler/_line_profiler.pyx +++ b/line_profiler/_line_profiler.pyx @@ -113,17 +113,18 @@ def label(code): return (code.co_filename, code.co_firstlineno, code.co_name) -cpdef _code_replace(code, co_code): +cpdef _code_replace(func, co_code): """ Implements CodeType.replace for Python < 3.8 """ + code = func.__code__ if hasattr(code, 'replace'): # python 3.8+ code = func.__code__.replace(co_code=co_code) else: # python <3.8 co = code - code = CodeType(co.co_argcount, co.co_kwonlyargcount, + code = type(code)(co.co_argcount, co.co_kwonlyargcount, co.co_nlocals, co.co_stacksize, co.co_flags, co_code, co.co_consts, co.co_names, co.co_varnames, co.co_filename, co.co_name, @@ -211,8 +212,7 @@ cdef class LineProfiler: self.dupes_map[code.co_code] += [code] # code hash already exists, so there must be a duplicate function. add no-op co_code = code.co_code + (9).to_bytes(1, byteorder=byteorder) * (len(self.dupes_map[code.co_code])) - CodeType = type(code) - code = _code_replace(code, co_code=co_code) + code = _code_replace(func, co_code=co_code) func.__code__ = code else: self.dupes_map[code.co_code] = [code] From 976da1ba0cb8fcf5464ff90bb79d5f8040f592f6 Mon Sep 17 00:00:00 2001 From: ichichkine Date: Tue, 4 Oct 2022 19:10:36 -0700 Subject: [PATCH 2/4] Move `enable_count` and `_c_last_time` to thread-local variables to fix race condition on threaded code --- line_profiler/_line_profiler.pyx | 33 +++++++++++++++++++++----------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/line_profiler/_line_profiler.pyx b/line_profiler/_line_profiler.pyx index 0491f5eb..02c503ee 100644 --- a/line_profiler/_line_profiler.pyx +++ b/line_profiler/_line_profiler.pyx @@ -6,6 +6,7 @@ from cpython.version cimport PY_VERSION_HEX from libc.stdint cimport int64_t from libcpp.unordered_map cimport unordered_map +import threading # long long int is at least 64 bytes assuming c99 ctypedef unsigned long long int uint64 @@ -175,18 +176,19 @@ cdef class LineProfiler: >>> self.print_stats() """ cdef unordered_map[int64, unordered_map[int64, LineTime]] _c_code_map - cdef unordered_map[int64, LastTime] _c_last_time + # Mapping between thread-id and map of LastTime + cdef unordered_map[int64, unordered_map[int64, LastTime]] _c_last_time cdef public list functions cdef public dict code_hash_map, dupes_map cdef public double timer_unit - cdef public long enable_count + cdef public object threaddata def __init__(self, *functions): self.functions = [] self.code_hash_map = {} self.dupes_map = {} self.timer_unit = hpTimerUnit() - self.enable_count = 0 + self.threaddata = threading.local() for func in functions: self.add_function(func) @@ -229,6 +231,14 @@ cdef class LineProfiler: self.functions.append(func) + property enable_count: + def __get__(self): + if not hasattr(self.threaddata, 'enable_count'): + self.threaddata.enable_count = 0 + return self.threaddata.enable_count + def __set__(self, value): + self.threaddata.enable_count = value + def enable_by_count(self): """ Enable the profiler if it hasn't been enabled before. """ @@ -263,7 +273,7 @@ cdef class LineProfiler: @property def c_last_time(self): - return self._c_last_time + return (self._c_last_time)[threading.get_ident()] @property def code_map(self): @@ -292,7 +302,7 @@ cdef class LineProfiler: line_profiler 4.0 no longer directly maintains last_time, but this will construct something similar for backwards compatibility. """ - c_last_time = self.c_last_time + c_last_time = (self._c_last_time)[threading.get_ident()] code_hash_map = self.code_hash_map py_last_time = {} for code, code_hashes in code_hash_map.items(): @@ -303,7 +313,7 @@ cdef class LineProfiler: cpdef disable(self): - self._c_last_time.clear() + self._c_last_time[threading.get_ident()].clear() unset_trace() def get_stats(self): @@ -344,9 +354,10 @@ PyObject *arg): block_hash = hash(get_frame_code(py_frame)) code_hash = compute_line_hash(block_hash, py_frame.f_lineno) if self._c_code_map.count(code_hash): + ident = threading.get_ident() time = hpTimer() - if self._c_last_time.count(block_hash): - old = self._c_last_time[block_hash] + if self._c_last_time[ident].count(block_hash): + old = self._c_last_time[ident][block_hash] line_entries = self._c_code_map[code_hash] key = old.f_lineno if not line_entries.count(key): @@ -356,12 +367,12 @@ PyObject *arg): if what == PyTrace_LINE: # Get the time again. This way, we don't record much time wasted # in this function. - self._c_last_time[block_hash] = LastTime(py_frame.f_lineno, hpTimer()) - elif self._c_last_time.count(block_hash): + self._c_last_time[ident][block_hash] = LastTime(py_frame.f_lineno, hpTimer()) + elif self._c_last_time[ident].count(block_hash): # We are returning from a function, not executing a line. Delete # the last_time record. It may have already been deleted if we # are profiling a generator that is being pumped past its end. - self._c_last_time.erase(self._c_last_time.find(block_hash)) + self._c_last_time[ident].erase(self._c_last_time[ident].find(block_hash)) return 0 From ba4f1adea0360dcc8a84dd990f6241975f4575c7 Mon Sep 17 00:00:00 2001 From: ichichkine Date: Tue, 4 Oct 2022 23:27:14 -0700 Subject: [PATCH 3/4] Read new time before other operations --- line_profiler/_line_profiler.pyx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/line_profiler/_line_profiler.pyx b/line_profiler/_line_profiler.pyx index 02c503ee..93edc552 100644 --- a/line_profiler/_line_profiler.pyx +++ b/line_profiler/_line_profiler.pyx @@ -354,8 +354,8 @@ PyObject *arg): block_hash = hash(get_frame_code(py_frame)) code_hash = compute_line_hash(block_hash, py_frame.f_lineno) if self._c_code_map.count(code_hash): - ident = threading.get_ident() time = hpTimer() + ident = threading.get_ident() if self._c_last_time[ident].count(block_hash): old = self._c_last_time[ident][block_hash] line_entries = self._c_code_map[code_hash] From a5d0e546150b896d585e7fc4fbfe9c0041c8ab21 Mon Sep 17 00:00:00 2001 From: ichichkine Date: Wed, 5 Oct 2022 00:51:26 -0700 Subject: [PATCH 4/4] In cases where there are multiple entries per line, only keep the one with highest # hits. This is the same behaviour used by `print_stats` --- line_profiler/_line_profiler.pyx | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/line_profiler/_line_profiler.pyx b/line_profiler/_line_profiler.pyx index 93edc552..09ef8dc6 100644 --- a/line_profiler/_line_profiler.pyx +++ b/line_profiler/_line_profiler.pyx @@ -328,8 +328,21 @@ cdef class LineProfiler: for entry in self.code_hash_map[code]: entries += list(cmap[entry].values()) key = label(code) - stats[key] = [(e["lineno"], e["nhits"], e["total_time"]) for e in entries] - stats[key].sort() + + entries = [(e["lineno"], e["nhits"], e["total_time"]) for e in entries] + # If there are multiple entries for a line, this will sort them by increasing # hits + entries.sort() + + # NOTE: v4.x may produce more than one entry per line. For example: + # 1: for x in range(10): + # 2: pass + # will produce a 1-hit entry on line 1, and 10-hit entries on lines 1 and 2 + # This doesn't affect `print_stats`, because it uses the last entry for a given line (line number is + # used a dict key so earlier entries are overwritten), but to keep compatability with other tools, + # let's only keep the last entry for each line + # Remove all but the last entry for each line + entries = list({e[0]: e for e in entries}.values()) + stats[key] = entries return LineStats(stats, self.timer_unit) @cython.boundscheck(False)