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

Switch to MacrotaskExecutor on JS #1522

Merged
merged 4 commits into from May 20, 2022

Conversation

armanbilge
Copy link
Contributor

@armanbilge armanbilge commented Dec 15, 2021

Closes #1519. Targeting series/3.x.

See https://github.com/scala-js/scala-js-macrotask-executor for more details.

Some notes:

  1. This required bumping to Scala.js 1.7.1.
  2. The monix-internal StandardContext this replaces actually did not suffer from the fairness problem that the default global ExecutionContext in Scala.js does. However, it seems like it would suffer from serious performance issues when used in browsers. Is anyone aware of this?

Comment on lines -42 to -47
private[this] val executeRef: js.Dynamic = {
if (js.typeOf(js.Dynamic.global.setImmediate) == "function")
js.Dynamic.global.setImmediate
else
js.Dynamic.global.setTimeout
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The MacrotaskExecutor is basically an advanced version of this conditional. First it looks for setImmediate (best performance). However, if that's not available, instead of giving up and falling back to setTimeout (very bad performance), MacrotaskExecutor attempts to implement a number of higher-performance polyfills instead.

@armanbilge
Copy link
Contributor Author

2. The monix-internal StandardContext this replaces actually did not suffer from the fairness problem that the default global ExecutionContext in Scala.js does. However, it seems like it would suffer from serious performance issues when used in browsers. Is anyone aware of this?

Yes, according to scala-js/scala-js#4129 (comment):

Monix has been using setImmediate in its Scheduler.global whenever available, ever since I ported it to Scala.js. Nobody complained.

@alexandru
Copy link
Member

I was thinking of doing window.postMessage, not sure if feasible

#1556

@alexandru
Copy link
Member

I took the liberty of merging with the base branch, and doing some fixes in build.sbt and in that test.

Note that in Monix I'd prefer to drop 3rd party dependencies in the core modules, however seeing this is an "official" Scala.js project, I guess it's all good.

Thanks for the PR. Will merge as soon as the build passes.

@armanbilge
Copy link
Contributor Author

Note that in Monix I'd prefer to drop 3rd party dependencies in the core modules, however seeing this is an "official" Scala.js project, I guess it's all good.

Yes, I read your blog post about the CE2/CE3 thing and 👍

The MacrotaskExecutor project should be very stable, ideally as stable as Scala.js itself. It's not part of Scala.js core largely due to their own policy/preference to avoid polyfills and non-standard APIs in core.

@alexandru alexandru enabled auto-merge (squash) May 20, 2022 16:30
@alexandru alexandru merged commit 711c966 into monix:series/3.x May 20, 2022
alexandru added a commit that referenced this pull request May 20, 2022
Co-authored-by: Alexandru Nedelcu <noreply@alexn.org>
alexandru added a commit that referenced this pull request May 20, 2022
Co-authored-by: Arman Bilge <armanbilge@gmail.com>
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

Successfully merging this pull request may close these issues.

None yet

2 participants