Allow speculative evaluation by asajeffrey · Pull Request #439 · w3c/css-houdini-drafts

asajeffrey · 2017-08-01T18:40:57Z

Currently, user agents can only invoke worklets with consistent snapshots of the state. This PR allows user agents to invoke worklet scripts with inconsistent state, as long as they discard the result.

The particular instance of this is paint worklets, since the CSS style values are known before the concrete size, so in Servo we speculate that the size hasn't changed (servo/servo#17810).

Fixes #406.

tschneidereit

Some comments based on an IRC conversation with @asajeffrey.

tschneidereit · 2017-08-01T19:01:52Z

+any time, and with any arguments, not just ones corresponding to
+consistent states of the user agent. The results of such speculative
+evaluations will be discarded, but may be cached for later use, thus
+increasing the concurrency between the user agent and worklet threads.


I think this paragraph is slightly misleading in two ways:

The methods won't be invoked with inconsistent state so much as with potential future state. The result will be used iff, when the time the invocation was done for has arrived, the actual state of the user agent matches what was passed in speculatively.

The results aren't discarded; they're not used immediately, but saved and hopefully used once the time the invocation was done for has arrived.

Good point, I tried rephrasing it.

tschneidereit · 2017-08-01T19:15:43Z

+
+Note: Although the image is discarded, it may still be cached, so subsequent invocations
+    of <<paint()>> may use the cached image.
+


Do we need to be this general about which value the function might be invoked with and when? Being more explicit about when and why a user agent might do this might be useful:

"The draw a paint image function may be speculatively invoked by the user agent with an expected value for |concreteObjectSize|. Once the |concreteObjectSize| is known, the resulting image either used if the speculation succeeded or discarded if the speculation failed."

I added text to the note: "Note: User agents may use whichever heuristics to speculate a possible future value for |concretObjectSize|, for example speculating that the size remains unchanged." I'm not sure that we want to say more about how to speculate in the normative text.

I replaced "discarded" by "not displayed", which is hopefully less confusing.

tschneidereit · 2017-08-01T20:47:56Z


-    2. If the <a>paint valid flag</a> for the |paintFunction| is <a>paint-valid</a> the user agent
+    2. If the <a>paint valid flag</a> for the |paintFunction| is <a>paint-valid</a>,
+        and the previous invocation had the same |concreteObjectSize|, the user agent


This implies that there can only ever be one speculative evaluation at a time. If we want to allow speculating multiple frames ahead, this should probably say something about checking for results of a speculative evaluation that match the input state.

All the discussion of the paint valid flag is dubious, perhaps we should just delete it? It's very single-threaded. This is probably a separate issue though.

There is a separate section which allows for the caching inside "invoke a paint callback", see step 10.

@bfgeek yes, this is why I was thinking we could just remove all the stuff about the paint valid flag, since we're already allowing caching inside "invoke a paint callback". Shall I submit a separate PR for that?

Yeah that sounds good, we still need to describe how the inputProperties invalidate that image on a box, but we can have that in prose vs. as an algorithm.

OK, I'll file an issue for this.

asajeffrey

Submitted another commit with some edits.

asajeffrey · 2017-08-01T21:15:22Z

+
+Note: Although the image is discarded, it may still be cached, so subsequent invocations
+    of <<paint()>> may use the cached image.
+


I added text to the note: "Note: User agents may use whichever heuristics to speculate a possible future value for |concretObjectSize|, for example speculating that the size remains unchanged." I'm not sure that we want to say more about how to speculate in the normative text.

I replaced "discarded" by "not displayed", which is hopefully less confusing.

asajeffrey · 2017-08-01T21:17:22Z


-    2. If the <a>paint valid flag</a> for the |paintFunction| is <a>paint-valid</a> the user agent
+    2. If the <a>paint valid flag</a> for the |paintFunction| is <a>paint-valid</a>,
+        and the previous invocation had the same |concreteObjectSize|, the user agent


All the discussion of the paint valid flag is dubious, perhaps we should just delete it? It's very single-threaded. This is probably a separate issue though.

asajeffrey · 2017-08-01T21:17:54Z

+any time, and with any arguments, not just ones corresponding to
+consistent states of the user agent. The results of such speculative
+evaluations will be discarded, but may be cached for later use, thus
+increasing the concurrency between the user agent and worklet threads.


Good point, I tried rephrasing it.

tschneidereit

Thanks, this looks great to me.

bfgeek

lgtm

bfgeek · 2017-08-09T11:25:33Z


-    2. If the <a>paint valid flag</a> for the |paintFunction| is <a>paint-valid</a> the user agent
+    2. If the <a>paint valid flag</a> for the |paintFunction| is <a>paint-valid</a>,
+        and the previous invocation had the same |concreteObjectSize|, the user agent


There is a separate section which allows for the caching inside "invoke a paint callback", see step 10.

bfgeek · 2017-08-09T11:26:35Z

+Speculative Evaluation {#speculative-evaluation}
+------------------------------------------------
+
+Some specifications which use worklets ([[css-paint-api-1]]) may


please reflow to 100 colwidth.

bfgeek · 2017-08-09T11:27:22Z

 The {{PaintSize}} object represents the size of the image that the author should draw. This is
 the <a>concrete object size</a> given by the user agent.

+The <a>draw a paint image</a> function may be speculatively invoked by


please reflow to 100 colwidth.

asajeffrey

Changed fill column.

asajeffrey · 2017-08-11T22:01:30Z

 The {{PaintSize}} object represents the size of the image that the author should draw. This is
 the <a>concrete object size</a> given by the user agent.

+The <a>draw a paint image</a> function may be speculatively invoked by


asajeffrey · 2017-08-11T22:02:48Z

+Speculative Evaluation {#speculative-evaluation}
+------------------------------------------------
+
+Some specifications which use worklets ([[css-paint-api-1]]) may


bfgeek

Looks great!

asajeffrey · 2017-11-10T00:40:01Z

Thanks!

Added text allowing speculative evaluation.

28084b3

tschneidereit suggested changes Aug 1, 2017

View reviewed changes

Responding to review comment.

5e309c7

asajeffrey commented Aug 1, 2017

View reviewed changes

tschneidereit approved these changes Aug 2, 2017

View reviewed changes

bfgeek requested changes Aug 9, 2017

View reviewed changes

Filled to column 100.

ac505d8

asajeffrey commented Aug 11, 2017

View reviewed changes

asajeffrey mentioned this pull request Aug 11, 2017

[css-paint-api] Unnecessary paint-valid flag? #447

Closed

bfgeek approved these changes Aug 15, 2017

View reviewed changes

bfgeek mentioned this pull request Aug 15, 2017

Custom Paint API to CR w3ctag/design-reviews#140

Closed

bfgeek added 2 commits November 9, 2017 08:47

Merge branch 'master' into speculative-evaluation

a0d3cba

Merge branch 'master' into speculative-evaluation

a2e7e63

bfgeek merged commit 36f9c91 into w3c:master Nov 9, 2017


		Note: Although the image is discarded, it may still be cached, so subsequent invocations
		of <<paint()>> may use the cached image.

Conversation

asajeffrey commented Aug 1, 2017

Uh oh!

tschneidereit left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

asajeffrey left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tschneidereit left a comment

Choose a reason for hiding this comment

Uh oh!

bfgeek left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

asajeffrey left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bfgeek left a comment

Choose a reason for hiding this comment

Uh oh!

asajeffrey commented Nov 10, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants