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)


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:3 Changed on 04/15/2017 at 02:50:16 PM by trev

  • Description modified (diff)

comment:4 Changed on 04/20/2017 at 08:22:02 AM by abpbot

A commit referencing this issue has landed:
Issue 5063 - [emscripten Make FilterNotifier calls more efficient]

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


Last edited on 10/08/2019 at 05:51:12 PM by kzar

Add Comment

Modify Ticket

Change Properties
as closed .
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from trev.
Note: See TracTickets for help on using tickets.