Opened on 03/30/2017 at 08:08:54 AM
Closed on 04/20/2017 at 08:22:28 AM
Last modified on 10/08/2019 at 05:51:12 PM
#5063 closed change (fixed)
[emscripten] Make FilterNotifier calls more efficient
| Reported by: | trev | Assignee: | trev |
|---|---|---|---|
| Priority: | P2 | Milestone: | |
| Module: | Core | Keywords: | |
| Cc: | Blocked By: | ||
| Blocking: | #4122, #5144 | Platform: | Unknown / Cross platform |
| Ready: | yes | Confidential: | no |
| Tester: | Unknown | Verified working: | no |
| Review URL(s): | |||
Description (last modified by trev)
Background
We currently call the JS class FilterNotifier whenever a filter or subscription property changes. However, we unnecessarily bloat the code size by creating a new EM_ASM block for each property. Also, we have the topic names in Emscripten even though these are constants - this wastes memory and requires a string conversion to be performed for each call. Finally, we currently have wrong topic names, e.g. filter.mDisabled rather than filter.disabled.
In addition, property change notifications transmit both the old and the new property value. This functionality is as good as unused - only FilterListener using the new value as far as I can see, and here it isn't really necessary.
What to change
Create a FilterNotifier C++ namespace providing the functionality to notify JavaScript code about changes - use the same EM_ASM block for all notifications. Define the topics as an enum and map the numerical topics to strings in JavaScript via custom bindings generator (#5062). Remove newValue and oldValue parameters.
Attachments (0)
Change History (6)
comment:1 Changed on 03/30/2017 at 10:09:54 AM by trev
- Review URL(s) modified (diff)
- Status changed from new to reviewing
comment:2 Changed on 04/15/2017 at 08:27:25 AM by trev
- Blocking 5144 added
- Summary changed from Make FilterNotifier calls more efficient to [emscripten] Make FilterNotifier calls more efficient
comment:4 Changed on 04/20/2017 at 08:22:02 AM by abpbot
comment:5 Changed on 04/20/2017 at 08:22:28 AM by trev
- Resolution set to fixed
- Status changed from reviewing to closed
comment:6 Changed on 09/14/2019 at 04:58:56 AM by tenmez190
spam
A commit referencing this issue has landed:
Issue 5063 - [emscripten Make FilterNotifier calls more efficient]