Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proposal improvement: detach listener and attach #10

Closed
betula opened this issue Aug 11, 2022 · 2 comments
Closed

proposal improvement: detach listener and attach #10

betula opened this issue Aug 11, 2022 · 2 comments

Comments

@betula
Copy link
Member

betula commented Aug 11, 2022

listen: (ev, fn) => (
    ev[0].indexOf(fn) < 0 && (ev[0] = ev[0].concat(fn)),
    () => {
      let i = ev[0].indexOf(fn)
      i > -1 && ev[0].splice(i, 1) // error here with iteration exactly all listeners in "ev" function
    }
  )

Reason:

  • If I detach another listener in the body of one listener, another listener should never be called.
  • If I attach the new listener in the body of one listener, the new listener should be called only on the next reaction.
@betula betula changed the title fix(0.9.1): unsubscribe and attach listener should mutate array of listeners fix(0.9.1): unsubscribe listener should mutate array of listeners Aug 11, 2022
@betula
Copy link
Member Author

betula commented Aug 11, 2022

The example above has an error. If I at first add a new listener, and at second remove the old one.

module.exports = {
  event: () => {
    let ev = (data) => {
      ev[1] ++;
      try {
          for (ev[3] = 0; i < ev[0].length; ev[3]++)
            ev[0][ev[3]](data)
      }
      finally {
          ev[1]--;
          if (!ev[1] && ev[2].length) {
            ev[0] = ev[0].concat(ev[2]);
            ev[2].length = 0;
          }
      }
    }
    ev[0] = []
    ev[1] = 0 // listen phase
    ev[2] = [] // new listeners
    ev[3] = 0 // current iteration index
    return ev
  },
  listen: (ev, fn) => (
    ev[0].indexOf(fn) < 0 && (ev[1] ? (ev[2].indexOf(fn) < 0 && ev[2].push(fn)) : ev[0].push(fn)),
    () => {
      let i = ev[0].indexOf(fn)
      i > -1 && (ev[1] && i <= ev[3] && ev[3]--, ev[0].splice(i, 1))
      i = ev[2].indexOf(fn)
      i > -1 && ev[2].splice(i, 1)
    }
  )
}

I should check its performance. Compare it with the original algorithm. We should understand how much we will miss.

@betula betula changed the title fix(0.9.1): unsubscribe listener should mutate array of listeners proposal improvement: detach listener and attach Aug 11, 2022
@betula
Copy link
Member Author

betula commented Aug 12, 2022

50% - 100% slower

➜ evemin git:(main) ✗ node perf_algo_test.js
origin
87.53 105.24 106.76 105.22 104.91 103.82 104.55 104.74 104.72 104.75
modified
158.49 159.25 157.6 156.54 157.11 156.58 156.76 157.47 156.16 157.14
600000000
➜ evemin git:(main) ✗ bun run perf_algo_test.js
origin
56.84 52.23 69.22 52.34 50.9 52.03 51.47 52.82 51.62 52.58
modified
98.65 123.16 97.3 97.64 98.09 98.2 99.02 98.56 98.6 97.76
600000000

@betula betula closed this as completed Aug 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant