feat(core): Manager::emit_filter and optimize serialization #7512

icambron · 2023-07-28T00:39:04Z

I emit a lot of events to multiple--but specific--windows, and one of the bottlenecks there is serialization. There are two problems:

There is no public API for emitting events to specific, multiple windows
The private implementation of emit_filter serializes on a per-window basis

This fixes both of those. A couple of design choices:

I figured emit_filter was the most flexible option, compared to taking a HashSet of window ids or similar. It's easy enough to pass |w| some_window_ids.contains(w.label()).
There's a small potential perf downside if none of the windows match filter, because we've already serialized the payload. I think this is a minor edge case but I could be wrong.

What kind of change does this PR introduce?

Does this PR introduce a breaking change?

Yes, and the changes were approved in issue #___
No

Checklist

When resolving issues, they are referenced in the PR's title (e.g fix: remove a typo, closes #___, #___)
A change file is added if any packages will require a version bump due to this PR per the instructions in the readme.
I have added a convincing reason for adding this feature, if necessary

Other information

icambron · 2023-07-28T00:41:18Z

core/tauri/src/manager.rs

    assert_event_name_is_valid(event);
    self
      .windows_lock()
      .values()
      .filter(|&w| filter(w))
-      .try_for_each(|window| window.emit_internal(event, source_window_label, payload.clone()))


I don't think this clone was ever needed. Even when we serialized once per window, it's just creating a string, so using &payload would have been better.

icambron · 2023-07-28T00:42:33Z

core/tauri/src/manager.rs

@@ -1102,12 +1102,13 @@ impl<R: Runtime> WindowManager<R> {
    S: Serialize + Clone,
    F: Fn(&Window<R>) -> bool,
  {
+    let emit_args = WindowEmitArgs::from(event, source_window_label, payload)?;


serialize once, emit many times

* added Manager::emit_filter * changed serialization so that it only happens once when sending to multiple window

icambron · 2023-08-04T20:04:01Z

core/tauri/src/manager.rs

@@ -1102,12 +1102,13 @@ impl<R: Runtime> WindowManager<R> {
    S: Serialize + Clone,
    F: Fn(&Window<R>) -> bool,
  {
+    let emit_args = WindowEmitArgs::from(event, source_window_label, payload)?;
    assert_event_name_is_valid(event);
    self
      .windows_lock()


One question I had here was whether it made more sense to call windows() instead of windows_lock(). This would have the effect of cloning all the windows (bad) but also of not holding the lock open (good). That seems to matter for two reasons. First, plausibly someone could write a filter that needs the lock, which would immediately cause a painful and unintuitive deadlock. Second, emitting might take a while, and ideally you could do other window things while that was happening.

When triggering events we usually clone the windows, it's safer. We should change this one.

@lucasfernog done

…ps#7512) Co-authored-by: Lucas Nogueira <lucas@tauri.studio>

* feat(tracing): add IPC tracing * span for deserialization * trace spans for IPC command handlers * fix spans usage * app tracing [skip ci] * window tracing * fix run never resolving all spans * fix draw not entered * change level * feat(core): Manager::emit_filter and optimize serialization (#7512) Co-authored-by: Lucas Nogueira <lucas@tauri.studio> * event spans * lint & fix tests * change eval to run sync * fix instrument * update wry * change separator * Update core/tauri/src/plugin.rs Co-authored-by: Jonas Kruckenberg <118265418+CrabNejonas@users.noreply.github.com> * Update core/tauri/src/window.rs Co-authored-by: Jonas Kruckenberg <118265418+CrabNejonas@users.noreply.github.com> * Update core/tauri/src/window.rs Co-authored-by: Jonas Kruckenberg <118265418+CrabNejonas@users.noreply.github.com> * Update core/tauri/src/window.rs Co-authored-by: Jonas Kruckenberg <118265418+CrabNejonas@users.noreply.github.com> * Update core/tauri/src/window.rs Co-authored-by: Jonas Kruckenberg <118265418+CrabNejonas@users.noreply.github.com> * instrument separators * remove on_event trace * skip all arguments on App::build tracing * ipc spans adjustments * delete change file * improve how request and response are added as values (serialize) * do not run evalScript sync on android :( freezes the app * wry 0.35.2 * add change file --------- Co-authored-by: Lucas Nogueira <lucas@tauri.studio> Co-authored-by: Jonas Kruckenberg <118265418+CrabNejonas@users.noreply.github.com>

icambron requested a review from a team as a code owner July 28, 2023 00:39

icambron changed the title ~~add Manager::emit_filter and optimize serialization~~ (feat) Manager::emit_filter and optimize serialization Jul 28, 2023

icambron commented Jul 28, 2023

View reviewed changes

icambron force-pushed the only-serialize-once branch from 183f8a6 to 454624a Compare July 28, 2023 00:56

icambron changed the title ~~(feat) Manager::emit_filter and optimize serialization~~ (feat: core) Manager::emit_filter and optimize serialization Jul 28, 2023

icambron changed the title ~~(feat: core) Manager::emit_filter and optimize serialization~~ feat(core) Manager::emit_filter and optimize serialization Jul 28, 2023

icambron changed the title ~~feat(core) Manager::emit_filter and optimize serialization~~ feat(core): Manager::emit_filter and optimize serialization Jul 28, 2023

icambron force-pushed the only-serialize-once branch from 371d99e to 63c50f0 Compare July 28, 2023 02:15

icambron changed the base branch from 1.x to dev July 28, 2023 02:15

icambron force-pushed the only-serialize-once branch from 63c50f0 to 371d99e Compare July 28, 2023 16:15

icambron changed the base branch from dev to 1.x July 28, 2023 16:15

add Manager::emit_filter and optimize serialization

7c88c3e

* added Manager::emit_filter * changed serialization so that it only happens once when sending to multiple window

icambron force-pushed the only-serialize-once branch from 5c4ebe9 to 7c88c3e Compare July 28, 2023 18:16

icambron commented Aug 4, 2023

View reviewed changes

clone windows on emit_filter

b2df221

lucasfernog previously approved these changes Aug 7, 2023

View reviewed changes

formatting

d8f2e49

icambron dismissed lucasfernog’s stale review via d8f2e49 August 7, 2023 22:06

Merge remote-tracking branch 'origin/1.x' into only-serialize-once

de94b35

lucasfernog approved these changes Aug 8, 2023

View reviewed changes

lucasfernog merged commit eeb6be5 into tauri-apps:1.x Aug 8, 2023
18 checks passed

lucasfernog-crabnebula added a commit to crabnebula-dev/tauri that referenced this pull request Sep 18, 2023

feat(core): Manager::emit_filter and optimize serialization (tauri-ap…

76da8a4

…ps#7512) Co-authored-by: Lucas Nogueira <lucas@tauri.studio>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(core): Manager::emit_filter and optimize serialization #7512

feat(core): Manager::emit_filter and optimize serialization #7512

icambron commented Jul 28, 2023 •

edited

icambron Jul 28, 2023 •

edited

icambron Jul 28, 2023

icambron Aug 4, 2023 •

edited

lucasfernog Aug 5, 2023

icambron Aug 7, 2023

feat(core): Manager::emit_filter and optimize serialization #7512

feat(core): Manager::emit_filter and optimize serialization #7512

Conversation

icambron commented Jul 28, 2023 • edited

What kind of change does this PR introduce?

Does this PR introduce a breaking change?

Checklist

Other information

icambron Jul 28, 2023 • edited

Choose a reason for hiding this comment

icambron Jul 28, 2023

Choose a reason for hiding this comment

icambron Aug 4, 2023 • edited

Choose a reason for hiding this comment

lucasfernog Aug 5, 2023

Choose a reason for hiding this comment

icambron Aug 7, 2023

Choose a reason for hiding this comment

icambron commented Jul 28, 2023 •

edited

icambron Jul 28, 2023 •

edited

icambron Aug 4, 2023 •

edited