Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Speed up on_trait_change, part II #1491

Merged
merged 4 commits into from
Aug 9, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 14 additions & 7 deletions traits/has_traits.py
Original file line number Diff line number Diff line change
Expand Up @@ -2658,20 +2658,27 @@ def on_trait_change(
if wrapper.equals(handler):
break
else:
listener = ListenerParser(name).listener
lnw = ListenerNotifyWrapper(
handler, self, name, listener, target
)
listeners.append(lnw)
listener.trait_set(
# The listener notify wrapper needs a reference to the
# listener, and the listener needs a (weak) reference to the
# wrapper. We first construct the wrapper with a listener of
# `None`, then construct the listener with its reference to the
# wrapper, then we replace the `None` listener with the correct
# one.
lnw = ListenerNotifyWrapper(handler, self, name, None, target)
listener = ListenerParser(
name,
handler=ListenerHandler(handler),
wrapped_handler_ref=weakref.ref(lnw),
type=lnw.type,
dispatch=dispatch,
).listener
listener.trait_set(
type=lnw.type,
priority=priority,
deferred=deferred,
Comment on lines +2675 to 2677
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not make the same changes to these three traits as well? Is that planned in a subsequent PR? type and deferred don't seem to have any static trait change handlers hooked up to them but priority does.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Subsequent PR. For type and deferred, it's messy, as you'll see when the next PR lands. :-) priority is mostly the same as in this PR, except that it's handled differently at ListenerGroup level.

)
lnw.listener = listener
listener.register(self)
listeners.append(lnw)

# A synonym for 'on_trait_change'
on_trait_event = on_trait_change
Expand Down
83 changes: 27 additions & 56 deletions traits/traits_listener.py
Original file line number Diff line number Diff line change
Expand Up @@ -126,12 +126,6 @@ def not_event(value):

class ListenerBase(HasPrivateTraits):

# The handler to be called when any listened to trait is changed:
# handler = Any

# The dispatch mechanism to use when invoking the handler:
# dispatch = Str

# Does the handler go at the beginning (True) or end (False) of the
# notification handlers list?
# priority = Bool( False )
Expand Down Expand Up @@ -498,24 +492,6 @@ def handle_error(self, obj, name, old, new):

# -- Event Handlers -------------------------------------------------------

def _handler_changed(self, handler):
""" Handles the **handler** trait being changed.
"""
if self.next is not None:
self.next.handler = handler

def _wrapped_handler_ref_changed(self, wrapped_handler_ref):
""" Handles the 'wrapped_handler_ref' trait being changed.
"""
if self.next is not None:
self.next.wrapped_handler_ref = wrapped_handler_ref

def _dispatch_changed(self, dispatch):
""" Handles the **dispatch** trait being changed.
"""
if self.next is not None:
self.next.dispatch = dispatch

def _priority_changed(self, priority):
""" Handles the **priority** trait being changed.
"""
Expand Down Expand Up @@ -861,15 +837,6 @@ def _get_value(self, name):

class ListenerGroup(ListenerBase):

#: The handler to be called when any listened-to trait is changed
handler = Property

#: A weakref 'wrapped' version of 'handler':
wrapped_handler_ref = Property

#: The dispatch mechanism to use when invoking the handler:
dispatch = Property

#: Does the handler go at the beginning (True) or end (False) of the
#: notification handlers list?
priority = ListProperty
Expand All @@ -891,26 +858,6 @@ class ListenerGroup(ListenerBase):
# The list of ListenerBase objects in the group
items = List(ListenerBase)

# -- Property Implementations ---------------------------------------------

def _set_handler(self, handler):
if self._handler is None:
self._handler = handler
for item in self.items:
item.handler = handler

def _set_wrapped_handler_ref(self, wrapped_handler_ref):
if self._wrapped_handler_ref is None:
self._wrapped_handler_ref = wrapped_handler_ref
for item in self.items:
item.wrapped_handler_ref = wrapped_handler_ref

def _set_dispatch(self, dispatch):
if self._dispatch is None:
self._dispatch = dispatch
for item in self.items:
item.dispatch = dispatch

# -- 'ListenerBase' Class Method Implementations --------------------------

def __repr__(self, seen=None):
Expand Down Expand Up @@ -993,7 +940,9 @@ def name(self):

# -- object Method Overrides ----------------------------------------------

def __init__(self, text):
def __init__(
self, text, handler=None, wrapped_handler_ref=None, dispatch=""
):
#: The text being parsed.
self.text = text

Expand All @@ -1003,6 +952,15 @@ def __init__(self, text):
#: The current parse index within the string.
self.index = 0

#: The handler to be called when any listened-to trait is changed.
self.handler = handler

#: A weakref 'wrapped' version of 'handler'.
self.wrapped_handler_ref = wrapped_handler_ref

#: The dispatch mechanism to use when invoking the handler.
self.dispatch = dispatch

#: The parsed listener.
self.listener = self.parse()

Expand All @@ -1027,7 +985,15 @@ def parse(self):
return ListenerItem(
name=match.group(1),
notify=match.group(2) == ".",
next=ListenerItem(name=match.group(3)),
next=ListenerItem(
name=match.group(3),
handler=self.handler,
wrapped_handler_ref=self.wrapped_handler_ref,
dispatch=self.dispatch,
),
handler=self.handler,
wrapped_handler_ref=self.wrapped_handler_ref,
dispatch=self.dispatch,
)

return self.parse_group(EOS)
Expand Down Expand Up @@ -1066,7 +1032,12 @@ def parse_item(self, terminator):
if name != "":
c = self.next

result = ListenerItem(name=name)
result = ListenerItem(
name=name,
handler=self.handler,
wrapped_handler_ref=self.wrapped_handler_ref,
dispatch=self.dispatch,
)

if c in "+-":
result.name += "*"
Expand Down