Speedup Observatory elements first time rendering

Speedup Observatory elements first time rendering

R=asiva@google.com, bkonyi@google.com

Committed: https://github.com/dart-lang/sdk/commit/7568ed5f69e384e1832757a6c27d343616626afb

[cbernaschina, 2017-08-14 15:35:04]

cbernaschina@google.com changed reviewers:
+ asiva@google.com, bkonyi@google.com, rmacnak@google.com

[bkonyi, 2017-08-14 20:26:43]

LGTM

[siva, 2017-08-14 20:27:19]

https://codereview.chromium.org/2995883002/diff/1/runtime/observatory/lib/src...
File runtime/observatory/lib/src/elements/helpers/rendering_queue.dart (right):

https://codereview.chromium.org/2995883002/diff/1/runtime/observatory/lib/src...
runtime/observatory/lib/src/elements/helpers/rendering_queue.dart:59: void
enqueue(RenderingTask r, {bool realtime: false}) {
instead of calling enqueue here with a parameter realtime why not just call
scheduleMicrotask(r.render) in the caller.

https://codereview.chromium.org/2995883002/diff/1/runtime/observatory/lib/src...
File runtime/observatory/lib/src/elements/helpers/rendering_scheduler.dart
(right):

https://codereview.chromium.org/2995883002/diff/1/runtime/observatory/lib/src...
runtime/observatory/lib/src/elements/helpers/rendering_scheduler.dart:108:
_firstTime = false;
instead of a flag _firstTime which is toggled why not just call
scheduleMicroTask(this.render) here.

[cbernaschina, 2017-08-14 21:08:49]

https://codereview.chromium.org/2995883002/diff/1/runtime/observatory/lib/src...
File runtime/observatory/lib/src/elements/helpers/rendering_queue.dart (right):

https://codereview.chromium.org/2995883002/diff/1/runtime/observatory/lib/src...
runtime/observatory/lib/src/elements/helpers/rendering_queue.dart:59: void
enqueue(RenderingTask r, {bool realtime: false}) {
On 2017/08/14 20:27:18, siva wrote:
> instead of calling enqueue here with a parameter realtime why not just call
> scheduleMicrotask(r.render) in the caller.

See new Patch.

The execution on a browser which supports custom elements is as follow:
 - the root component schedules the first task and initializes the rendering
stage
 - all the children schedule an extra task that is executed just after the
parent one
   (due to the fact that the render methods are synchronous all the components
will render during a single Microtask)

The execution on a browser with partial support of the custom elements is as
follow:
 - the root component schedules the first task and initializes the rendering
stage
 - all the children schedule an extra task that is executed via Microtask,
possibly packing groups of them together.

Forwarding the decision to the enqueue method guarantees that the extra
Microtasks are created just as a fallback and not as part of the normal
behavior.

https://codereview.chromium.org/2995883002/diff/1/runtime/observatory/lib/src...
File runtime/observatory/lib/src/elements/helpers/rendering_scheduler.dart
(right):

https://codereview.chromium.org/2995883002/diff/1/runtime/observatory/lib/src...
runtime/observatory/lib/src/elements/helpers/rendering_scheduler.dart:108:
_firstTime = false;
On 2017/08/14 20:27:18, siva wrote:
> instead of a flag _firstTime which is toggled why not just call
> scheduleMicroTask(this.render) here.

ditto

[siva, 2017-08-14 22:35:01]

lgtm

https://codereview.chromium.org/2995883002/diff/20001/runtime/observatory/lib...
File runtime/observatory/lib/src/elements/helpers/rendering_queue.dart (right):

https://codereview.chromium.org/2995883002/diff/20001/runtime/observatory/lib...
runtime/observatory/lib/src/elements/helpers/rendering_queue.dart:59: void
enqueue(RenderingTask r, {bool realtime: false}) {
why not rename realtime to waitForBarrier to make the code more readable.

https://codereview.chromium.org/2995883002/diff/20001/runtime/observatory/lib...
File runtime/observatory/lib/src/elements/helpers/rendering_scheduler.dart
(right):

https://codereview.chromium.org/2995883002/diff/20001/runtime/observatory/lib...
runtime/observatory/lib/src/elements/helpers/rendering_scheduler.dart:35: bool
_firstTime = true;
ditto here, _waitForBarrier = false;

[cbernaschina, 2017-08-14 23:04:14]

https://codereview.chromium.org/2995883002/diff/20001/runtime/observatory/lib...
File runtime/observatory/lib/src/elements/helpers/rendering_queue.dart (right):

https://codereview.chromium.org/2995883002/diff/20001/runtime/observatory/lib...
runtime/observatory/lib/src/elements/helpers/rendering_queue.dart:59: void
enqueue(RenderingTask r, {bool realtime: false}) {
On 2017/08/14 22:35:01, siva wrote:
> why not rename realtime to waitForBarrier to make the code more readable.

Done.

https://codereview.chromium.org/2995883002/diff/20001/runtime/observatory/lib...
File runtime/observatory/lib/src/elements/helpers/rendering_scheduler.dart
(right):

https://codereview.chromium.org/2995883002/diff/20001/runtime/observatory/lib...
runtime/observatory/lib/src/elements/helpers/rendering_scheduler.dart:35: bool
_firstTime = true;
On 2017/08/14 22:35:01, siva wrote:
> ditto here, _waitForBarrier = false;

Done.

[cbernaschina, 2017-08-14 23:04:36]

Description was changed from

==========
Speedup Observatory elements first time rendering
==========

to

==========
Speedup Observatory elements first time rendering

R=asiva@google.com, bkonyi@google.com

Committed:
https://github.com/dart-lang/sdk/commit/7568ed5f69e384e1832757a6c27d343616626afb
==========

[cbernaschina, 2017-08-14 23:04:37]

Committed patchset #3 (id:40001) manually as
7568ed5f69e384e1832757a6c27d343616626afb (presubmit successful).