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

Run all JS from a task scheduled on the task-queue? #24737

Closed
gterzian opened this issue Nov 14, 2019 · 3 comments
Closed

Run all JS from a task scheduled on the task-queue? #24737

gterzian opened this issue Nov 14, 2019 · 3 comments
Labels

Comments

@gterzian
Copy link
Member

@gterzian gterzian commented Nov 14, 2019

We currently have a task-queue:

pub struct TaskQueue<T> {

To which tasks from all task-sources are scheduled through.

pub trait TaskSource {

However, not all JS runs via a task-source, for example timers are run directly when receiving a message from the scheduler at

fn handle_timer_event(&self, timer_event: TimerEvent) {

UI events from the compositor are also dispatched directly via

fn handle_event(&self, pipeline_id: PipelineId, event: CompositorEvent) {

So while this is OK, one could argue that these represent distinct task-queues and that only the relative ordering inside a task-queue matters, it can be somewhat surprising. See for example #24664 (comment)

Should we make some effort to route everything through the task-queue? It could then also be a central point where prioritization could be taken into account, in fact it already is, as the task-queue potentially throttles certain task-sources, see

TaskSourceName::PerformanceTimeline => return true,

Also, the task-queue has some logic dealing with inactive pipelines, whereas currently timers and compositor events each need to perform their own checks, see for example:

if window.Closed() {

and

if w.Closed() {

@gterzian
Copy link
Member Author

@gterzian gterzian commented Nov 14, 2019

On the downside, it does mean handling a message from the compositor, and then queuing a task(which is essentially sending another message to the script-thread itself). So perhaps the overhead is not worth it for UI events?

Actually, since those message come-in over IPC anyway, we could instead of routing them to a single port using

let control_port = ROUTER.route_ipc_receiver_to_new_crossbeam_receiver(state.control_port);

we could have a more specific routing, for example by plugging certain messages into the task-queue, for example the ConstellationControlMsg::SendEvent could be routed directly into the task-queue, as opposed to into the main select of the script-thread at

recv(self.control_port) -> msg => FromConstellation(msg.unwrap()),

@gterzian
Copy link
Member Author

@gterzian gterzian commented Nov 15, 2019

I've done some reading:

  1. It seems UI events don't always have to be disptached inside a task, in fact, it seems they usually aren't dispatched in one at all. See https://w3c.github.io/uievents/
  2. Timers should be dispatched from inside a task, using the timer-task-source, so that something we could implement.
@gterzian
Copy link
Member Author

@gterzian gterzian commented Nov 17, 2019

The actionable element is found at #24747

@gterzian gterzian closed this Nov 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant
You can’t perform that action at this time.