Bug 35022 – [V8] Refactor ScheduledAction to support different kinds of actions

• Status: RESOLVED
• Resolution: WONTFIX
• Component: WebCore JavaScript

[Dominic Cooney]

ScheduledAction is hard-wired to invoke V8 functions; separating the ScheduledAction interface and implementation which invokes a V8 function permits other kinds of scheduled actions.

[Dominic Cooney]

Created attachment 48872 [details]
Patch

[Dimitri Glazkov (Google)]

How does this look performance-wise? Any visible regressions? I am ok with the change if it doesn't introduce perf regressions.

[Sam Weinig]

Why is this useful?

[Dominic Cooney]

(In reply to comment #2)
> How does this look performance-wise? Any visible regressions? I am ok with the
> change if it doesn't introduce perf regressions.

I wrote a microbenchmark, first just calling execute (with no context => execute doesn't find a V8 context and exits); then creating and destroying the (V8)ScheduledAction as well. The difference looks to be within noise, <0.5%. I ran the Chromium perf test suite.

(In reply to comment #3)
> Why is this useful?

V8 has two scheduled actions--one with a window context; one with a worker context. I'm separating the interface so that we can have different kinds of scheduled actions in Chromium without having to wind them together in the same file with if statements.

[Dmitry Titov]

Please see bug 23229 for alternative idea.

I guess the motivation here is to remove multiple execute() methods and the 'if' that checks the type of context... So perhaps the plan is to have another patch after this one, which will split the V8ScheduledAction into 2 classes, one for Document context and another for Worker context, is that your intention?

If that's true, perhaps indeed moving execute() to SEC is a better way then creating more classes?

[Dmitry Titov]

Comment on attachment 48872 [details]
Patch

In a separate discussion, it was said that this is a part of a feature that is being worked on but is not ready enough yet. So it's hard to see how (or when) the change will be used in WebKit, so it's hard to get the best possible feedback. I'm going to r- it for now. When we have more 'complete' chunk or more info that can be evaluated by the open project reviewers, we should look at it again.

[Anders Carlsson]

V8 is gone from WebKit.