About message passing, DOM events and the benchmark

[Greg Weng]

Hello everyone,

TL;DR: we should implement an individual message passing way to avoid
binding application messages with DOM events. This is good for both
performance and desiging.

Tim and I always wonder how slower/faster to use JavaScript implement event
emitter (compare to the DOM event). This matters because the current way we
use DOM events are not all for their original intentions, like passing and
handling user's behavior, like 'click' or 'touchmove'. We also use DOM
event as a channel to pass messages among components within one app
(window.dispatchEvent/addEventListener). For example, lockscreen's `unlock`
and `lock` events, and window manager's `showwindow` or `hidewindow`. These
events are unrrelevant to DOM elements, and the window can be totally
replaced by a 'purer' version of event emitter.

So we choose the EventEmitter.js as a pure JS emitter (why invent our
wheels while there're so many on the market?), and wrote a simple test for
it on jsPerf:

http://jsperf.com/eventemitter-js-vs-dom-events/5

*Caveats*: I wrote this in a hurry, so if this test is not fair, please
patch it or correct me.

This test would create 100 callbacks on both EventEmitter and window, and
the thing it completes is to get the current timestamp and calculate the
interval. I think it performance a basic manipulation that 1. declar local
variable 2. use some API 3. do some calc. Even though I didn't let write
the result out.

Result: DOM version is slower than EventEmitter version significantly. You
can run the demo to get yourown data on your local console. My data shows
Ops/sec is closed to:

EE: 17,100
DOM: 4,300

On Firefox Nightly 29. Google Chrome given similar results.

In conclusion, I think we need a individual way to pass messages. Even this
is hard to be true to have a native version, like what Node.js' module can
use, in our JS engine or browser APIs (discussions, proposal, quarrel,
fighting, discussion, proposal...), we can still do this firstly in Gaia.
And even if my benchmark isn't fair enough and DOM events are actually
faster than JS EE, we may still need to do this. Because mixing application
messages with DOM events are actually binding two unrrelevant notificaitons
together, and it's cause ambiguous design.

(Tim mentioned some features should be done in such EE, like follow the
current `preventDefault` way we used to cancel or supress some events. I
think these are issues need to be solved, but not obstacles that can't be
conquered).

--
<http://about.me/snowmantw>Greg Weng

http://about.me/snowmantw

*Understand y f = f [ y f ] ; lose last remaining non-major friend*

* -- Anonymous*

[Julien Wajsberg]

Hey Greg,

Just a question: is performance currently an issue ? I think we're
sending events once in a while only, and for these cases performance is
irrelevant.

[Greg Weng]

I'm not expert in performance, but I guess if we reduce events and
callbacks bounded on window or DOM, it would be good for some edge case
like scrolling (usually with overwhelming scrolling events), or touch
moving.

BTW: I forgot to attach the version of one callback in EE and DOM:

http://jsperf.com/eventemitter-js-vs-dom-events/6

[Julien Wajsberg]

I'm really not sure performance should be the reason to change this.

But there are a lot of other valid reasons :)
* better control over the implementation (for example you might want
asynchronous events)
* less risk of misusing an existing DOM event name

Obviously the downside is changing something that works using native
capabilities ;)

--
Julien

[Kevin Grandon]

We've mostly been having performance battles with app startup time. Including another library would slightly impact startup times, so we should understand what those tradeoffs would be.

It seems like we have a good case for making a proposal to enhance the platform here, or perhaps look into the performance characteristics of dom events.

Best,
Kevin

_______________________________________________
dev-gaia mailing list
dev-...@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-gaia

[Fabrice Desré]

On 01/03/2014 02:29 AM, Greg Weng wrote:

> Result: DOM version is slower than EventEmitter version significantly. You
> can run the demo to get yourown data on your local console. My data shows
> Ops/sec is closed to:
>
> EE: 17,100
> DOM: 4,300
>
> On Firefox Nightly 29. Google Chrome given similar results.

But on a zte open the speedup for EE is only about 2x. Also you're
measuring some overhead that may be irrelevant when something more
substantial happens in the callback. We had some performance issues with
the mozChromeEvents and they were unrelated to the dispatching code, but
only due to the callbacks themselves.

> (Tim mentioned some features should be done in such EE, like follow the
> current `preventDefault` way we used to cancel or supress some events. I
> think these are issues need to be solved, but not obstacles that can't be
> conquered).

The more features you'll add to your EE the more they will look like dom
events. I'm not super impressed by the speed up so far, and I'd like to
know more about the reasons for a switch. How many events do we have?
how many listeners typically? do we use
bubbling/capturing/preventDefault/stopPropagation ?

I understand you believe that this would be a cleaner design, and that
may be very true, but that's the only obvious benefit I think.

Fabrice
--
Fabrice Desr�
b2g team
Mozilla Corporation

[Vivien Nicolas]

We are fixing scrolling in the platform with APZC. See bug 909877.
In the future the pointer-events speak should even let us get rid of
GestureDectector.js so don't worry about scrolling / touch moving.

On 03/01/2014 11:55, Greg Weng wrote:
> I'm not expert in performance, but I guess if we reduce events and
> callbacks bounded on window or DOM, it would be good for some edge case
> like scrolling (usually with overwhelming scrolling events), or touch
> moving.
>
> BTW: I forgot to attach the version of one callback in EE and DOM:
>
> http://jsperf.com/eventemitter-js-vs-dom-events/6

[Ben Kelly]

On 1/3/2014 5:29 AM, Greg Weng wrote:
> Result: DOM version is slower than EventEmitter version significantly. You
> can run the demo to get yourown data on your local console. My data shows
> Ops/sec is closed to:
>
> EE: 17,100
> DOM: 4,300

Looking at the code at git.io/ee it appears EventEmitter.js emits all
events synchronously. I believe DOM events will be asynchronous. I
think we should be cautious about converting async events to sync events
as we could easily break functionality.

Also, I'm curious how much of the speed difference is due to the async
vs sync behavior.

Thanks for investigating this!

Ben

[Fabrice Desré]

On 01/06/2014 07:52 AM, Ben Kelly wrote:
> On 1/3/2014 5:29 AM, Greg Weng wrote:
>> Result: DOM version is slower than EventEmitter version significantly.
>> You
>> can run the demo to get yourown data on your local console. My data shows
>> Ops/sec is closed to:
>>
>> EE: 17,100
>> DOM: 4,300
>
> Looking at the code at git.io/ee it appears EventEmitter.js emits all
> events synchronously. I believe DOM events will be asynchronous. I
> think we should be cautious about converting async events to sync events
> as we could easily break functionality.

No, DOM event dispatching is synchronous. See
http://www.w3.org/TR/2007/WD-DOM-Level-3-Events-20071221/events.html#Events-flow

and in particular: "The DOM event model is reentrant. Event listeners
may perform actions that cause additional events to be dispatched. Such
events are handled in a synchronous manner, the event propagation that
causes the event listener to be triggered will resume only after the
event dispatch of the new event is completed."

[Rick Waldron]

On Mon, Jan 6, 2014 at 10:52 AM, Ben Kelly <bke...@mozilla.com> wrote:

> On 1/3/2014 5:29 AM, Greg Weng wrote:
>
>> Result: DOM version is slower than EventEmitter version significantly. You
>> can run the demo to get yourown data on your local console. My data shows
>> Ops/sec is closed to:
>>
>> EE: 17,100
>> DOM: 4,300
>>
>
> Looking at the code at git.io/ee it appears EventEmitter.js emits all
> events synchronously. I believe DOM events will be asynchronous. I think
> we should be cautious about converting async events to sync events as we
> could easily break functionality.
>

> Also, I'm curious how much of the speed difference is due to the async vs
> sync behavior.
>

An additional point to consider is that "EventEmitter" and the various
knock-offs that exist are all using soon-to-be antiquated or just plain
poor design patterns:

1. Junk code for old platforms:
https://github.com/Wolfy87/EventEmitter/blob/master/EventEmitter.js#L32-L41
2. Stores events on the instance object of whatever class has inherited it:
https://github.com/Wolfy87/EventEmitter/blob/master/EventEmitter.js#L446-L449
3. Exposes its implementation details:
https://github.com/Wolfy87/EventEmitter/blob/master/EventEmitter.js#L110(one
of many examples)
4. Many duplicate method names:
- https://github.com/Wolfy87/EventEmitter/blob/master/EventEmitter.js#L395
- https://github.com/Wolfy87/EventEmitter/blob/master/EventEmitter.js#L360
- https://github.com/Wolfy87/EventEmitter/blob/master/EventEmitter.js#L405(slightly
different, but preferably so)

Rick

[Alive]

Hi,

Thanks for bringing this up,
but currently I have the same opinion with Julien and Fabrice,
this is not a top priority stuff to do.

DOM events may not be a perfect solution but we could do something to make this cleaner,
see
https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/system.js#L36
https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/app_window.js#L767
we could re-use a publish function to wrap all event dispatch call and replace the DOM event dispatch if we really need someday.

And with DOM events, I could easily have public events and private events,
For non-specific events, dispatch on global object.
For object-specific events, dispatch on specific element.
See https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/app_window.js#L782
and https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/app_window.js#L319

If dispatching on window is a performance kill(is it?), we could choose to dispatch on another element.

So my proposed action would be replace all DOM events dispatching with single publish function and wait and see if we really need to discard this.

I won't worry about sync and async, we could always setTimeout in the sync callback. We already do this in some parts of new Window Management,
but be careful to the 'state' in the timer.

-Alive

> Result: DOM version is slower than EventEmitter version significantly. You
> can run the demo to get yourown data on your local console. My data shows
> Ops/sec is closed to:
>
> EE: 17,100
> DOM: 4,300
>

> On Firefox Nightly 29. Google Chrome given similar results.
>

> In conclusion, I think we need a individual way to pass messages. Even this
> is hard to be true to have a native version, like what Node.js' module can
> use, in our JS engine or browser APIs (discussions, proposal, quarrel,
> fighting, discussion, proposal...), we can still do this firstly in Gaia.
> And even if my benchmark isn't fair enough and DOM events are actually
> faster than JS EE, we may still need to do this. Because mixing application
> messages with DOM events are actually binding two unrrelevant notificaitons
> together, and it's cause ambiguous design.
>

> (Tim mentioned some features should be done in such EE, like follow the
> current `preventDefault` way we used to cancel or supress some events. I
> think these are issues need to be solved, but not obstacles that can't be
> conquered).
>

> --
> <http://about.me/snowmantw>Greg Weng
>
> http://about.me/snowmantw
>
> *Understand y f = f [ y f ] ; lose last remaining non-major friend*
>
> * -- Anonymous*

> _______________________________________________
> dev-gaia mailing list
> dev-...@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-gaia

--
Alive C. Kuo, Front-end Engineer, FirefoxOS, MoCo. Taiwan, Taipei office.
al...@mozilla.com

[Rick Waldron]

On Mon, Jan 6, 2014 at 11:27 AM, Alive <al...@mozilla.com> wrote:

> Hi,
>
> Thanks for bringing this up,
> but currently I have the same opinion with Julien and Fabrice,
> this is not a top priority stuff to do.
>
> DOM events may not be a perfect solution but we could do something to make
> this cleaner,
> see
>
> https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/system.js#L36
>
> https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/app_window.js#L767
> we could re-use a publish function to wrap all event dispatch call and
> replace the DOM event dispatch if we really need someday.


> And with DOM events, I could easily have public events and private events,
> For non-specific events, dispatch on global object.
> For object-specific events, dispatch on specific element.
> See
> https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/app_window.js#L782
> and
> https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/app_window.js#L319
>
> If dispatching on window is a performance kill(is it?), we could choose to
> dispatch on another element.
>
> So my proposed action would be replace all DOM events dispatching with
> single publish function and wait and see if we really need to discard this.
>

One of the examples above is special casing the dispatch of "internal"
events prior to "external" events—not all applications need this, so what
will happen to the code that _does_ need it?

Rick

[Greg Weng]

I didn't mean we must or should consider to use this library. The only
reason I choose it was I want to find a simple and suitable library to help
me to finish the benchmark (compare to jQuery, which is the second choice
in my list).


2014/1/7 Rick Waldron <waldro...@gmail.com>

>
>
>
> On Mon, Jan 6, 2014 at 10:52 AM, Ben Kelly <bke...@mozilla.com> wrote:
>
>> On 1/3/2014 5:29 AM, Greg Weng wrote:
>>

>>> Result: DOM version is slower than EventEmitter version significantly.
>>> You
>>> can run the demo to get yourown data on your local console. My data shows
>>> Ops/sec is closed to:
>>>
>>> EE: 17,100
>>> DOM: 4,300
>>>
>>

>> Looking at the code at git.io/ee it appears EventEmitter.js emits all
>> events synchronously. I believe DOM events will be asynchronous. I think
>> we should be cautious about converting async events to sync events as we
>> could easily break functionality.
>>
>> Also, I'm curious how much of the speed difference is due to the async vs
>> sync behavior.
>>
>
> An additional point to consider is that "EventEmitter" and the various
> knock-offs that exist are all using soon-to-be antiquated or just plain
> poor design patterns:
>
> 1. Junk code for old platforms:
> https://github.com/Wolfy87/EventEmitter/blob/master/EventEmitter.js#L32-L41
> 2. Stores events on the instance object of whatever class has inherited
> it:
> https://github.com/Wolfy87/EventEmitter/blob/master/EventEmitter.js#L446-L449
> 3. Exposes its implementation details:
> https://github.com/Wolfy87/EventEmitter/blob/master/EventEmitter.js#L110(one of many examples)
> 4. Many duplicate method names:
> -
> https://github.com/Wolfy87/EventEmitter/blob/master/EventEmitter.js#L395
> -
> https://github.com/Wolfy87/EventEmitter/blob/master/EventEmitter.js#L360
> -
> https://github.com/Wolfy87/EventEmitter/blob/master/EventEmitter.js#L405(slightly different, but preferably so)
>
> Rick
>
>

[Greg Weng]

I don't know whether private/public event by different event dispatcher is
good or not. I've seen two different designation to handle message passing
like this: one is to collect all events to a unique hub, like our 'window',
and then dispatch it. Another is to have small event hubs scattered about
the codebase like hooking and dispatching on different DOM elements. I
favour the centralized way according to the experience of designing the
similar system in my own JavaScript framework, which shown that there would
be lots of troubles to manage these private hubs, especially when the
message passing become the major way to communicate with components.

And yes, we don't need to do this in this time. The way I prefer to ask
questions or discuss possible improvements is to send it public as soon as
possible. I believe in this way we can keep this in our mind, and to
implement it in the right time.


2014/1/7 Alive <al...@mozilla.com>

> Hi,
>
> Thanks for bringing this up,
> but currently I have the same opinion with Julien and Fabrice,
> this is not a top priority stuff to do.
>
> DOM events may not be a perfect solution but we could do something to make
> this cleaner,
> see
>
> https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/system.js#L36
>
> https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/app_window.js#L767
> we could re-use a publish function to wrap all event dispatch call and
> replace the DOM event dispatch if we really need someday.
>
> And with DOM events, I could easily have public events and private events,
> For non-specific events, dispatch on global object.
> For object-specific events, dispatch on specific element.
> See
> https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/app_window.js#L782
> and
> https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/app_window.js#L319
>
> If dispatching on window is a performance kill(is it?), we could choose to
> dispatch on another element.
>
> So my proposed action would be replace all DOM events dispatching with
> single publish function and wait and see if we really need to discard this.
>

> Result: DOM version is slower than EventEmitter version significantly. You
> can run the demo to get yourown data on your local console. My data shows
> Ops/sec is closed to:
>
> EE: 17,100
> DOM: 4,300
>

> On Firefox Nightly 29. Google Chrome given similar results.
>
> In conclusion, I think we need a individual way to pass messages. Even this
> is hard to be true to have a native version, like what Node.js' module can
> use, in our JS engine or browser APIs (discussions, proposal, quarrel,
> fighting, discussion, proposal...), we can still do this firstly in Gaia.
> And even if my benchmark isn't fair enough and DOM events are actually
> faster than JS EE, we may still need to do this. Because mixing application
> messages with DOM events are actually binding two unrrelevant notificaitons
> together, and it's cause ambiguous design.
>
> (Tim mentioned some features should be done in such EE, like follow the
> current `preventDefault` way we used to cancel or supress some events. I
> think these are issues need to be solved, but not obstacles that can't be
> conquered).
>

> --
> <http://about.me/snowmantw>Greg Weng
>
> http://about.me/snowmantw
>
> *Understand y f = f [ y f ] ; lose last remaining non-major friend*
>
> * -- Anonymous*
>

> _______________________________________________
> dev-gaia mailing list
> dev-...@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-gaia
>
>
> --
> Alive C. Kuo, Front-end Engineer, FirefoxOS, MoCo. Taiwan, Taipei office.
> al...@mozilla.com
>
>
>
>
>

[Fabrice Desré]

Hi Greg,

On 01/06/2014 04:06 PM, Greg Weng wrote:

> And yes, we don't need to do this in this time. The way I prefer to ask
> questions or discuss possible improvements is to send it public as soon as
> possible. I believe in this way we can keep this in our mind, and to
> implement it in the right time.

And that is really great that you're spending time to write proposals
and listen to people's feedback. I hope you don't feel bad when they are
not accepted!

Fabrice
--
Fabrice Desré
b2g team
Mozilla Corporation

[Jan Jongboom]

I miss having a simple way to create an EventEmitter quite a lot of times and pulling in libraries is very annoying. Couldn't we expose the interface without DOM bindings to normal javascript? Put it in the standard? (Might be more for the webapi mailing list but soit).

[Julien Wajsberg]

Le 07/01/2014 01:20, Fabrice Desré a écrit :
> Hi Greg,
>
> On 01/06/2014 04:06 PM, Greg Weng wrote:
>
>> And yes, we don't need to do this in this time. The way I prefer to ask
>> questions or discuss possible improvements is to send it public as soon as
>> possible. I believe in this way we can keep this in our mind, and to
>> implement it in the right time.
> And that is really great that you're spending time to write proposals
> and listen to people's feedback. I hope you don't feel bad when they are
> not accepted!
>

Same here. We (and certainly I ;) ) can sometimes sound rude, especially
in written form, especially when english is not our native language.
Thanks a lot for taking time to make things better :)

--
Julien

[Greg Weng]

It would be perfect if we can provide something like Node.js' native
EventEmitter in web API or Javascript language. But I think it would take
long time to discuss and implement & test, so I just mentioned the llibrary
in pure JavaScript.


2014/1/7 Jan Jongboom <janjo...@gmail.com>

> _______________________________________________
> dev-gaia mailing list
> dev-...@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-gaia
>

[Rick Waldron]

On Tue, Jan 7, 2014 at 11:05 AM, Greg Weng <snow...@gmail.com> wrote:

> It would be perfect if we can provide something like Node.js' native
> EventEmitter in web API or Javascript language. But I think it would take
> long time to discuss and implement & test, so I just mentioned the llibrary
> in pure JavaScript.
>

Coincidentally, Anne Van Kesteren and I intend to write the proposal which
I will be championing for ES7 to add an "EventEmitter" module to the
standard language modules.

Rick

[Greg Weng]

The interesting thing is ES itself (without DOM) is *not* a message passing
driven language (from the PL aspect). Not like Smalltalk which adopted the
message passing as its first-class feature, ES never had this feature. So
if you success (I really hope so), you may change the ES' programming
paradigm.


2014/1/8 Rick Waldron <waldro...@gmail.com>

[Greg Weng]

Today John told me that we *already* used pure JavaScript event emitter in
the new Camera app...

https://github.com/mozilla-b2g/gaia/blob/master/apps/camera/js/vendor/evt.js

So, this was already happened! And we may try this in other new or
refactoring code.

Beside that, I don't know why this code is not in shared/js, although we're
discussing to migrate from shared/js to bower package system in the build
script discussions[1]. Obviously it is a standalone library can be included
and used by other apps. This make me worry about that we would never stop
to inventing the same wheel even in our own project.

And he also found that there is a mixin 'library' which I mentioned earlier
in another thread[2]....

https://github.com/mozilla-b2g/gaia/blob/master/apps/camera/js/utils/mixin.js

And the 'padLeft' which has been implemented 11 times and has been
discussed in the thread...

https://github.com/mozilla-b2g/gaia/blob/master/apps/camera/js/utils/padleft.js

Things like this upset me, because this is apparently a serious problem,
but people just do their tiny version of 'private' libraries and others
don't know what had happened, and then invent it again and again. It make
me feel it's urgent to push the shared libraries splitting plan to let
these code can be included, used, organized and manage dependencies well.

[1]:
https://groups.google.com/d/msg/mozilla.dev.gaia/nq7FgDbS4QM/ovGf346yDzsJ
[2]: https://groups.google.com/forum/#!topic/mozilla.dev.gaia/6lQbHAbGmSs


2014/1/8 Greg Weng <snow...@gmail.com>

> The interesting thing is ES itself (without DOM) is *not* a message
> passing driven language (from the PL aspect). Not like Smalltalk which
> adopted the message passing as its first-class feature, ES never had this
> feature. So if you success (I really hope so), you may change the ES'
> programming paradigm.
>
>
> 2014/1/8 Rick Waldron <waldro...@gmail.com>
>
>>
>>
>>
>> On Tue, Jan 7, 2014 at 11:05 AM, Greg Weng <snow...@gmail.com> wrote:
>>
>>> It would be perfect if we can provide something like Node.js' native
>>> EventEmitter in web API or Javascript language. But I think it would take
>>> long time to discuss and implement & test, so I just mentioned the
>>> llibrary
>>> in pure JavaScript.
>>>
>>
>> Coincidentally, Anne Van Kesteren and I intend to write the proposal
>> which I will be championing for ES7 to add an "EventEmitter" module to the
>> standard language modules.
>>
>> Rick
>>
>>
>
>
>

[James Burke]

On Thu, Jan 9, 2014 at 7:22 PM, Greg Weng <snow...@gmail.com> wrote:

> Today John told me that we *already* used pure JavaScript event emitter in
> the new Camera app...
>
>
> https://github.com/mozilla-b2g/gaia/blob/master/apps/camera/js/vendor/evt.js
>
> So, this was already happened! And we may try this in other new or
> refactoring code.
>
>

That looks like it is based on the one in email, which I introduced. I have
been hesitant to advocate for it because ideally I would like to see the
emit be async, but initially feared event loop clogging that would slow
down actual app code completion. I would really like to see a microtask
backed emit for that reason. Maybe once promises land. In the meantime,
note the synchronous emit.

It was also purposely named something other than EventEmitter because it
does not try to be a faithful member of that type, particularly the
latest/latestOnce and emitWhenListener use.

James

[Fred Lin]

I think we can do it unobtrusively, just need some general rules for it

1. moving third party (vendor) libraries to shared/vendor
2. have central document to note what's in our shared folder, so every developer can check if they can reuse some of it

Then build script can do the trick whatever we use nothing/npm/bower to fetch vendor libraries.


regards
--
Fred



----- 原始郵件 -----
寄件人: "Greg Weng" <snow...@gmail.com>
收件人: "Rick Waldron" <waldro...@gmail.com>
副本: "dev-gaia" <dev-...@lists.mozilla.org>, "Jan Jongboom" <janjo...@gmail.com>
寄件箱: 2014年1 月10日, 星期五 上午 11:22:50
標題: Re: About message passing, DOM events and the benchmark

Today John told me that we *already* used pure JavaScript event emitter in
the new Camera app...

https://github.com/mozilla-b2g/gaia/blob/master/apps/camera/js/vendor/evt.js

So, this was already happened! And we may try this in other new or
refactoring code.

> --
> <http://about.me/snowmantw>Greg Weng
>
> http://about.me/snowmantw
>
> *Understand y f = f [ y f ] ; lose last remaining non-major friend*
>
> * -- Anonymous*
>
>


--
<http://about.me/snowmantw>Greg Weng

http://about.me/snowmantw

*Understand y f = f [ y f ] ; lose last remaining non-major friend*

* -- Anonymous *

[Mike Pennisi]

Further evidencing the need to avoid duplication, Clock implements yet
another event emitter:

https://github.com/mozilla-b2g/gaia/blob/8862901daa7b9befce33d1d679091f26437386d1/apps/clock/js/emitter.js

On 01/10/2014 12:49 AM, James Burke wrote:
> On Thu, Jan 9, 2014 at 7:22 PM, Greg Weng <snow...@gmail.com> wrote:
>

>> Today John told me that we *already* used pure JavaScript event emitter in
>> the new Camera app...
>>
>>
>> https://github.com/mozilla-b2g/gaia/blob/master/apps/camera/js/vendor/evt.js
>>
>> So, this was already happened! And we may try this in other new or
>> refactoring code.
>>
>>

> That looks like it is based on the one in email, which I introduced. I have
> been hesitant to advocate for it because ideally I would like to see the
> emit be async, but initially feared event loop clogging that would slow
> down actual app code completion. I would really like to see a microtask
> backed emit for that reason. Maybe once promises land. In the meantime,
> note the synchronous emit.
>
> It was also purposely named something other than EventEmitter because it
> does not try to be a faithful member of that type, particularly the
> latest/latestOnce and emitWhenListener use.
>
> James

[Greg Weng]

It seems we would have 11 implementations again... (like padLeft)


2014/1/11 Mike Pennisi <mi...@bocoup.com>

[Greg Weng]

And since we don't and may be unable to force every reviewer to report
those possible duplicated or can be extracted code like this, it would be
difficult to find these issues. I do know some reviewer would notice this,
like leaving comment to tell the committer to use Template.js instead of
inventing some templating code, but may be we need more eyes on this thing.

Maybe I can regularly spend sometime to check recent commits to see if
there're any code like these. However, since we now has no formal way to
push this (a code quality team?), I can only try my best.


2014/1/11 Greg Weng <snow...@gmail.com>

[Fabrice Desré]

On 01/10/2014 08:03 PM, Greg Weng wrote:
> And since we don't and may be unable to force every reviewer to report
> those possible duplicated or can be extracted code like this, it would be
> difficult to find these issues. I do know some reviewer would notice this,
> like leaving comment to tell the committer to use Template.js instead of
> inventing some templating code, but may be we need more eyes on this thing.
>
> Maybe I can regularly spend sometime to check recent commits to see if
> there're any code like these. However, since we now has no formal way to
> push this (a code quality team?), I can only try my best.

That's the very role of peers and module owners to ensure code quality
and notice this kind of mistakes. If that doesn't happen, we need to fix
that by making sure that any potential reviewer is aware of what should
be used from shared code.

[Mike Pennisi]

After recognizing this need, maintainers likely first checked the
"shared/" directory for a generic solution. Not finding one, approving
an app-specific solution was probably the best response. This was the
case for Clock, and I suspect E-mail and Camera both went through a
similar process.

The per-app module organization means that this may happen repeatedly.
This is good justification for the mailing list (and discussions like
these), where we can recognize patterns that arise from real
requirements (not just, "wouldn't an event emitter be cool?") and
coalesce on robust solutions. I'd be willing to accept duplication in
the short term for that.

Still, it would be helpful if the acceptance criteria for the "shared/"
directory were documented (or more visible).

Mike

[Fabrice Desré]

On 01/11/2014 11:41 AM, Mike Pennisi wrote:
> Still, it would be helpful if the acceptance criteria for the "shared/"
> directory were documented (or more visible).

The peer for shared/js is djf according to
https://wiki.mozilla.org/Modules/FirefoxOS, and there are other people
for subparts of shared/. So getting djf to agree on moving something
here is the acceptance criteria.

[Kevin Grandon]

Hey guys,

Luckily none of these issues are hard to solve! These conversations are good, but let's make sure something actionable comes out of them.

I am working on the following bugs. Once the code lands in shared/ it should be very easy to migrate the apps over to use it. Feel free to submit a pull request.

11 declarations of padLeft or pad functions - Bug 934025

Multiple copies of alameda.js in the codebase - Bug 958856


Relevant to this thread, I have filed the following bug, which is up for grabs:

Multiple implementations of event emitter in the codebase - Bug 958864


If I missed something, please file a bug. I'm looking forward to making our codebase best-in-class with you all.

Best,
Kevin


----- Original Message -----
From: "Fabrice Desré" <fab...@mozilla.com>
To: "Mike Pennisi" <mi...@bocoup.com>, "Greg Weng" <snow...@gmail.com>
Cc: "dev-gaia" <dev-...@lists.mozilla.org>

Fabrice Desré
b2g team
Mozilla Corporation

[James Burke]

One thing that has bothered me about /shared is that its current state is
forced on all apps, whether they have tested it or not.

Ideally whoever is doing the upgrade for a shared library would test all
apps that use it, but I am not sure that is a reasonable thing to expect. I
for one avoid putting things in shared because I do not want to be on the
hook for vouching that it works well in apps that I may not understand how
they are constructed.

I prefer the model where an app could decide to get a certain version of
commit version of a library, and the app could decide when to do upgrades.
The library author would just broadcast any new changes (or app authors
would watch the lib for changes), and it is up to app owners to decide when
to upgrade.

That gets into the other threads about componentized app structures and the
use of a package manager or `repo` tools, which is a bit of a hairball.

To keep things simple, as an experiment, instead of using /shared for
alameda, I suggest projects that use it try using volo[1] instead to
install any new versions they want for it. alameda already has its own
repo, so it benefits from being able to try this easily — there is
effectively only one canonical “location” for it, the github repo.

volo is nice in that it just installs single file JS files when it can, and
stamps a package.json with the version, branch, or commit hash used to
fetch it, so there is a trail to find where the current version in the repo
came from, but not a lot of extra package dependency file debris.

It is also nice because it is a package manager tool that is only used once
by the developer when installing or upgrading a dependency. The dependency
is committed to the app’s directory. So no issues with travis usage, npm
availability and such. Or, volo could even be avoided if whoever does the
pull request to update an app’s version of the file, just update the
package.json volo.dependencies entry with the version used and add the new
file.

I will update the alameda /shared bug[2] with info on how projects can use
volo to get the latest version. It is also a good candidate since I will
soon have a 0.2.0 alameda with an updated internal promise library[3], so
the upgrade experience can be tried out for projects that want to try it.

Disclaimer: I am the primary author of volo.

[1] http://volojs.org
[2] https://bugzilla.mozilla.org/show_bug.cgi?id=958856
[3] https://bugzilla.mozilla.org/show_bug.cgi?id=958675

James