Create new event with emitter object to simplify code

[Tyriar]

@jerch thoughts? I think this is pretty neat 🙂

Runtime performance seems the same

[jerch]

This def. looks easier to use 👍

Yes perf would be my main concern here for these reasons:

• heavy getter usage (prolly not an issue anymore with ES2015?)
• heavy "closured" code, e.g. binding of Emitter.fire and Emitter.dispose onto Emitter.event capturing Emitter 3 times (not even sure if GC can cleanup this fast as Emitter is captured circular on Emitter.event?)

I remember older browser engines to have issues with GC and circular refs, imho this needs a memory burn test from millions of events and whether dispose still gets them cleaned up. Also perf-wise - prolly needs tests on different engines (no clue how Firefox keeps up with that).

Lastly I think this approach makes the Emitter itself unreachable from code symbols - during some debug tests way back I needed to inspect the listeners, which is now not possible anymore?

At this point I wonder, if a more flat code structure as some sort of Event class still giving access to emitter internals would avoid all those issues/concerns?

[Tyriar]

heavy getter usage (prolly not an issue anymore with ES2015?)

Yes I don't think the getter thing is a problem in newer TS targets, but also this isn't a getting but a regular value property (value is used, not get), so it should work similarly.

heavy "closured" code, e.g. binding of Emitter.fire and Emitter.dispose onto Emitter.event capturing Emitter 3 times (not even sure if GC can cleanup this fast as Emitter is captured circular on Emitter.event?)

I'll try test

Lastly I think this approach makes the Emitter itself unreachable from code symbols - during some debug tests way back I needed to inspect the listeners, which is now not possible anymore?

I exposed _listeners as well, purely to help with debugging/testing:

At this point I wonder, if a more flat code structure as some sort of Event class still giving access to emitter internals would avoid all those issues/concerns?

We could have done this before by just exposing Emitter such that IEvent would be { event: Event }, but then it's ugly to use: onMyEvent.event(listener).

[Tyriar]

Some tests below, it seems as good as or better based on the results. Provided the tests are doing the right thing 🙂

---

Init + listen to 1 million events performance

Before:

  private _testEvents(): void {
    const emitter = new EventEmitter<string>();
    const event = emitter.event;
    for (let i = 0; i < 1000000; i++) {
      event(e => e);
    }
  }

After:

  private _testEvents(): void {
    const ev = initEvent<string>();
    for (let i = 0; i < 1000000; i++) {
      ev(e => e);
    }
  }

---

Init + listen to 1 million events + fire performance

Before:

  private _testEvents(): void {
    const emitter = new EventEmitter<string>();
    const event = emitter.event;
    for (let i = 0; i < 1000000; i++) {
      event(e => e);
    }
    emitter.fire('foo');
  }

After:

  private _testEvents(): void {
    const ev = initEvent<string>();
    for (let i = 0; i < 1000000; i++) {
      ev(e => e);
    }
    ev.fire('foo');
  }

---

Memory after forced GC without dispose

Before:

  private _emitter: IEventEmitter<string> | undefined = new EventEmitter<string>();
  private _testEvents(): void {
    const event = this._emitter!.event;
    for (let i = 0; i < 1000000; i++) {
      event!(e => e);
    }
    this._emitter!.fire('foo');
  }

Heap snapshot: 52.6MB

After:

  private _ev = initEvent<string>();
  private _testEvents(): void {
    for (let i = 0; i < 1000000; i++) {
      this._ev(e => e);
    }
    this._ev.fire('foo');
  }

Heap snapshot: 45.3MB

---

Memory after forced GC with dispose

Before:

  private _emitter: IEventEmitter<string> | undefined = new EventEmitter<string>();
  private _testEvents(): void {
    let event: IEvent<string> | undefined = this._emitter!.event;
    for (let i = 0; i < 1000000; i++) {
      event!(e => e);
    }
    this._emitter!.fire('foo');
    setTimeout(() => {
      this._emitter!.dispose();
      this._emitter = undefined;
      event = undefined;
    }, 1000);
  }

Heap snapshot: ~23-28MB

After:

  private _ev: IEventWithEmitter<string> | undefined = initEvent<string>();
  private _testEvents(): void {
    for (let i = 0; i < 1000000; i++) {
      this._ev!(e => e);
    }
    this._ev!.fire('foo');
    setTimeout(() => {
      this._ev!.dispose();
      this._ev = undefined;
    }, 1000);
  }

Heap snapshot: 15.7MB

[jerch]

Yepp, looks better than the old variant 👍. Not sure what magic they use to spot unreachable cycling refs in GC w'o perf hit, maybe they can mark whole cycles as unreachable pretty fast w'o going into bad complexity from expensive graph algos.

[Tyriar]

Will merge then, we can always tweak later on if there are problems. So much nicer to write and read like this