Adding tasks during flush

[janschoenherr]

If one tries to enqueue a measure:mutate pair during a flush, it is possible that the mutate is executed before the measure. This PR tries to resolve that.

[coveralls]

Coverage remained the same at 98.4% when pulling b8d54309234b2a3da9a3f2d2204879f541c9d8cd on janschoenherr:patch-1 into 99beda37e8c6f7932e8c1bb79572fca7f1852161 on wilsonpage:master.

[wilsonpage]

This is a relatively big change which will change timing in and potentially break apps. It essentially means nested tasks will always run in the next frame.

Could you give more details on what scenario this fixes. I think we'd need a test case.

Appreciate your time :)

[janschoenherr]

Thanks for this project, we are using it in uikit. In UIkit we are using components that update each other. Components update by registering to the fastdom queue.

So if Component A registers a measure/mutate pair and during its mutate phase signals Component B to update, something like this might happen.

Fastdom Flush
A -> measure
A -> mutate -> triggers B to update
B -> mutate
Fastdom Flush
B -> measure

B registers a measure/mutate pair during the mutating phase of the flush. And thereby B's mutate executes before its measure

For me it was important, that a mutate cannot happen before the corresponding measure. That would potentially render wrong results.

There is another drawback to the proposed PR. You can no longer "cancel" the mutate from the measure phase.

I know this is a breaking change. Just wanted to discuss the issue. There might be other solutions, but I am not sure on how A could safely wait until the flush is done, before registering its next update.

[wilsonpage]

@janschoenherr thanks for the more detailed explanation. I'm still struggling to fully grasp, are you able to show the same scenario with a code sample?

[generalov]

I've hacked around this PR. It looked interesting but it's just braking a several tests. It seems I've reproduced the issue. But it may be just an expected behaviour if writing of the measure and mutate operations just one after the other can't provide the ordering.

They are called in single RAF: { measure, mutate }:

fastdom.measure(()=>0);
fastdom.mutate(()=>0);

They are called in single RAF: { measure, measure, mutate }:

fastdom.measure(()=>{
  fastdom.measure(()=>0);
  fastdom.mutate(()=>0);
});

They are called in two RAFs: { mutate, mutate }, { measure }:

fastdom.mutate(()=>{
  fastdom.measure(()=>0);
  fastdom.mutate(()=>0);
});

Note the swapping of measure and mutate calls order. It may seem conterintuitive case in comparing to examples the above. Is it an issue?

Anyway the callbacks could be used to guarantee the ordering. They will be run in expected order and two RAFs: { mutate }, { measure, muate }:

fastdom.mutate(()=>{
  fastdom.measure(()=>{
     fastdom.mutate(()=>0);
  });
});

[generalov]

The ordering may be preserverd in mutate() callbacks too https://github.com/generalov/fastdom/commit/0359c27 , but I'm not shure anymore that it is needed.

[wilsonpage]

@generalov thanks for digging into this! This is in-fact the designed behaviour. The reason for this is because:

• A. We attempt to complete the work in the least amount of frames (so it's completed ASAP).
• B. There isn't a use-case I could think of for the below example.

fastdom.measure(()=>{
  fastdom.measure(()=>0);
  fastdom.mutate(()=>0);
});

[wilsonpage]

For the record I can see how this may appear to be counter intuitive. This is a definite breaking change, if the community wants it I'd accept it and bump the version.

[generalov]

A. We attempt to complete the work in the least amount of frames (so it's completed ASAP).

Thank you. It's really very interesting. Could you please explain reasons why the reads and writes tasks queues are executed just once a frame? Some tasks could append new tasks to queues. So the writes tasks is being executed in the same frame, but the reads tasks will be scheduled to the next frame. Why the fastdome doesn't aim to execute all tasks in the current frame untill both queues will be empty?

[janschoenherr]

I think, what would somewhat lessen the impact of the PR might be, if the measures and mutates would be moved to the next RAF only, once the mutation phase has started? In that case that would most likely be the intended behaviour anyway, otherwise it would cause the browser to reflow again.

I've updated the above given example.

[wilsonpage]

Why the fastdome doesn't aim to execute all tasks in the current frame untill both queues will be empty?

A frame as a very short period of time. The browser only really has enough time to do one render cycle per frame. As soon as the DOM is mutated, it's 'dirty' and a render cycle is scheduled (restyle, layout and paint). If you perform a measure task while the DOM is dirty the scheduled render cycle must be brought forward and executed synchronously, which is costly.

Fastdom's job is to take the given tasks and execute them at the best times, that work with (not against) the browser's render model. Shoving all the work into one frame would result in render cycles per frame, which will cause jank.

[wilsonpage]

So just to clarify

Current behaviour:

fastdom.measure(() => { // frame 1
  fastdom.measure(() => {}); // frame 1
  fastdom.mutate(() => {});  // frame 2
});

Behaviour with suggested change:

fastdom.measure(() => { // frame 1
  fastdom.measure(() => {}); // frame 2 <--
  fastdom.mutate(() => {});  // frame 2
});

[wilsonpage]

I'm going to ask the wise @rowanbeentje. He has a lot of experience running fastdom in production. @rowanbeentje thoughts on the above suggestion?

[janschoenherr]

Current behaviour:

fastdom.measure(() => { // frame 1
  fastdom.measure(() => {}); // frame 1
  fastdom.mutate(() => {});  // frame 2
});

No, here everything is executed in the correct order in the first frame: http://codepen.io/anon/pen/oBrXYa

The problem is with:

fastdom.mutate(() => { // frame 1
  fastdom.measure(() => {}); // frame 2
  fastdom.mutate(() => {});  // frame 1
});

http://codepen.io/anon/pen/OWeVmP