Skip to content

Commit 68def13

Browse files
mergify[bot]jd
andauthored
refactor(profiling): remove lock from _ThreadLink (#4147) (#4151)
The _ThreadLink data structure historically used a lock to make sure there was no threads overlapping each other. For example, this could happen: 1. Main thread from the application starts a trace and `link_object` is being called 2. Stack profiler would run and clear the thread list or get an spans from the list Removing the lock makes it possible that, while the stack profiler thread clears the mapping, a user's thread links a thread to a span. That's a trade-off we are willing to accept for performance and safeness reason. (cherry picked from commit 39bc44f) Co-authored-by: Julien Danjou <[email protected]>
1 parent e0bdccc commit 68def13

File tree

1 file changed

+15
-16
lines changed

1 file changed

+15
-16
lines changed

ddtrace/profiling/_threading.pyx

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -69,19 +69,14 @@ class _ThreadLink(_thread_link_base):
6969
# Key is a thread_id
7070
# Value is a weakref to an object
7171
_thread_id_to_object = attr.ib(factory=dict, repr=False, init=False, type=typing.Dict[int, _weakref_type])
72-
# Note that this lock has to be reentrant as spans can be activated unexpectedly in the same thread
73-
# ex. during a gc weakref finalize callback
74-
_lock = attr.ib(factory=nogevent.RLock, repr=False, init=False, type=nogevent.RLock)
7572

7673
def link_object(
7774
self,
7875
obj # type: _T
7976
):
8077
# type: (...) -> None
8178
"""Link an object to the current running thread."""
82-
# Since we're going to iterate over the set, make sure it's locked
83-
with self._lock:
84-
self._thread_id_to_object[nogevent.thread_get_ident()] = weakref.ref(obj)
79+
self._thread_id_to_object[nogevent.thread_get_ident()] = weakref.ref(obj)
8580

8681
def clear_threads(self,
8782
existing_thread_ids, # type: typing.Set[int]
@@ -92,11 +87,17 @@ class _ThreadLink(_thread_link_base):
9287
9388
:param existing_thread_ids: A set of thread ids to keep.
9489
"""
95-
with self._lock:
96-
# Iterate over a copy of the list of keys since it's mutated during our iteration.
97-
for thread_id in list(self._thread_id_to_object.keys()):
98-
if thread_id not in existing_thread_ids:
99-
del self._thread_id_to_object[thread_id]
90+
# This code clears the thread/object mapping by clearing a copy and swapping it in an atomic operation This is
91+
# needed to be able to have this whole class lock-free and avoid concurrency issues.
92+
# The fact that it is lock free means we might lose some accuracy, but it's worth the trade-off for speed and simplicity.
93+
new_thread_id_to_object_mapping = self._thread_id_to_object.copy()
94+
# Iterate over a copy of the list of keys since it's mutated during our iteration.
95+
for thread_id in list(new_thread_id_to_object_mapping):
96+
if thread_id not in existing_thread_ids:
97+
del new_thread_id_to_object_mapping[thread_id]
98+
99+
# Swap with the new list
100+
self._thread_id_to_object = new_thread_id_to_object_mapping
100101

101102
def get_object(
102103
self,
@@ -108,8 +109,6 @@ class _ThreadLink(_thread_link_base):
108109
:param thread_id: The thread id.
109110
:return: The attached object.
110111
"""
111-
112-
with self._lock:
113-
obj_ref = self._thread_id_to_object.get(thread_id)
114-
if obj_ref is not None:
115-
return obj_ref()
112+
obj_ref = self._thread_id_to_object.get(thread_id)
113+
if obj_ref is not None:
114+
return obj_ref()

0 commit comments

Comments
 (0)