Bug 179451 - Async iteration should only fetch the next method once and add feature flag
Summary: Async iteration should only fetch the next method once and add feature flag
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Keith Miller
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-11-08 15:18 PST by Keith Miller
Modified: 2017-11-15 14:19 PST (History)
10 users (show)

See Also:


Attachments
Patch (48.65 KB, patch)
2017-11-08 15:37 PST, Keith Miller
ggaren: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Keith Miller 2017-11-08 15:18:28 PST
Async iteration should only fetch the next method once and add feature flag
Comment 1 Keith Miller 2017-11-08 15:37:50 PST
Created attachment 326396 [details]
Patch
Comment 2 GSkachkov 2017-11-09 14:20:40 PST
Comment on attachment 326396 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=326396&action=review

> Source/JavaScriptCore/ChangeLog:9
> +        the expected behavior of the proposal.

Looks good to me, but not sure that we have tests that cover this case.
Comment 3 Geoffrey Garen 2017-11-10 12:36:41 PST
Comment on attachment 326396 [details]
Patch

r=me
Comment 4 Keith Miller 2017-11-13 15:10:09 PST
Committed r224787: <https://trac.webkit.org/changeset/224787>
Comment 5 Per Arne Vollan 2017-11-14 07:29:19 PST
Should we also enable this feature for CMake builds?
Comment 6 Saam Barati 2017-11-14 09:21:53 PST
(In reply to Per Arne Vollan from comment #5)
> Should we also enable this feature for CMake builds?

Yeah.

Keith, what was the motivation for adding a build flag here?
Comment 7 Keith Miller 2017-11-14 09:46:12 PST
(In reply to Saam Barati from comment #6)
> (In reply to Per Arne Vollan from comment #5)
> > Should we also enable this feature for CMake builds?
> 
> Yeah.

Yeah, I'll add one for CMake.

> 
> Keith, what was the motivation for adding a build flag here?

The feature has not reached stage 4 yet so we don't want to accidentally ship the feature.
Comment 8 Saam Barati 2017-11-14 11:18:26 PST
(In reply to Keith Miller from comment #7)
> (In reply to Saam Barati from comment #6)
> > (In reply to Per Arne Vollan from comment #5)
> > > Should we also enable this feature for CMake builds?
> > 
> > Yeah.
> 
> Yeah, I'll add one for CMake.
> 
> > 
> > Keith, what was the motivation for adding a build flag here?
> 
> The feature has not reached stage 4 yet so we don't want to accidentally
> ship the feature.

But we have a runtime option already, right?
Comment 9 Radar WebKit Bug Importer 2017-11-15 09:39:29 PST
<rdar://problem/35562169>
Comment 10 Michael Catanzaro 2017-11-15 12:27:58 PST
(In reply to Saam Barati from comment #8)
> But we have a runtime option already, right?

Where are JSC runtime options defined?

(In reply to Keith Miller from comment #7)
> The feature has not reached stage 4 yet so we don't want to accidentally
> ship the feature.

Is any precaution taken to ensure it's not already shipping on CMake ports? E.g. is it gated behind ENABLE(DEVELOPER_MODE)? For WebKit-level features, that's easy to accomplish by setting defaultValue: DEFAULT_EXPERIMENTAL_FEATURES_ENABLED in WebPreferences.yaml. But I don't know how this is handled for JSC.
Comment 11 Keith Miller 2017-11-15 14:19:16 PST
(In reply to Michael Catanzaro from comment #10)
> Where are JSC runtime options defined?

They are in Source/JavaScriptCore/runtime/Options.*

> Is any precaution taken to ensure it's not already shipping on CMake ports?
> E.g. is it gated behind ENABLE(DEVELOPER_MODE)? For WebKit-level features,
> that's easy to accomplish by setting defaultValue:
> DEFAULT_EXPERIMENTAL_FEATURES_ENABLED in WebPreferences.yaml. But I don't
> know how this is handled for JSC.

This feature was only added in the last month or so. Unless someone shipped in the last month they won't have the feature. I don't think JSC has anything like experimental features in WebKit.