Skip to content

Conversation

@bdash
Copy link
Contributor

@bdash bdash commented Jul 10, 2025

The ExportsTreeView now asks the model to pause / resume updates when its visibility changes. When paused, all notifications from the view are ignored. When resumed, notifications set a flag to indicate that an update is needed and resume the update timer if it is not already active. The timer is stopped after an update is processed.

There's some extra complexity here to avoid emitting a signal for every BinaryView notification that is processed. These notifications are typically generated on background threads. The overhead of emitting a signal and it being routed to the main thread adds up given the number of notifications involved when loading a large binary. This manifests on macOS as time being spent inside CFRunLoopWakeUp.

@bpotchik
Copy link
Member

Implementation looks good.

Additional context (informational only):

SidebarWidgets and Views include a quiescing interface (enableRefreshTimer) that provides a base timer and control signals for when widgets are shown/hidden in the UI. The current approach using showEvent/hideEvent works well for this scenario since the widget resets the model rather than collecting data from each BinaryDataNotification.

Additionally, it's possible to completely unregister from BinaryViewNotifications when the widget is hidden, then re-register and rebuild the model when it becomes visible again. I only mention this because, occasionally when opening a file and view navigation fails, the triage view is gratuitously created but never shown. It would then be an idle consumer of all notifications it registered for. There are some nuances to implementing this, so just providing it as information.

Copy link
Member

@plafosse plafosse left a comment

Choose a reason for hiding this comment

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

I'm approving but I left a couple of comments on some potential improvements. Address those comments if you agree with them and then merge.

The `ExportsTreeView` now asks the model to pause / resume updates when
its visibility changes. When paused, all notifications from the view are
ignored. When resumed, notifications set a flag to indicate that an
update is needed and resume the update timer if it is not already
active. The timer is stopped after an update is processed.

There's some extra complexity here to avoid emitting a signal for every
`BinaryView` notification that is processed. These notifications are
typically generated on background threads. The overhead of emitting a
signal and it being routed to the main thread adds up given the number
of notifications involved when loading a large binary.
@bdash bdash force-pushed the test_triage_summary_perf branch from 8ce4c6d to ebac408 Compare July 10, 2025 23:08
@bdash bdash merged commit ebac408 into dev Jul 10, 2025
4 of 5 checks passed
@bdash bdash deleted the test_triage_summary_perf branch July 10, 2025 23:12
@emesare emesare added this to the Helion milestone Jul 23, 2025
Sign up for free to 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.

5 participants