RESOLVED FIXED 206140
[JSC] Remove IsDone from JSArrayIterator 
https://bugs.webkit.org/show_bug.cgi?id=206140
Summary [JSC] Remove IsDone from JSArrayIterator
Yusuke Suzuki
Reported 2020-01-12 01:10:35 PST
[JSC] Remove IsDone from JSArrayIterator
Attachments
Patch (12.55 KB, patch)
2020-01-12 01:12 PST, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (12.06 KB, patch)
2020-01-12 15:02 PST, Yusuke Suzuki 		keith_miller: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Yusuke Suzuki
Comment 1 2020-01-12 01:12:08 PST
Created attachment 387461 [details] Patch
Devin Rousso
Comment 2 2020-01-12 07:08:23 PST
Comment on attachment 387461 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=387461&action=review > Source/JavaScriptCore/inspector/JSInjectedScriptHost.cpp:-549 > - clone->internalField(JSArrayIterator::Field::IsDone).set(vm, clone, iteratorObject->internalField(JSArrayIterator::Field::IsDone).get()); I think we should still show this in Web Inspector, as it’s useful for developers to know when debugging.
Yusuke Suzuki
Comment 3 2020-01-12 15:00:18 PST
Comment on attachment 387461 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=387461&action=review The test failures look unrelated. >> Source/JavaScriptCore/inspector/JSInjectedScriptHost.cpp:-549 >> - clone->internalField(JSArrayIterator::Field::IsDone).set(vm, clone, iteratorObject->internalField(JSArrayIterator::Field::IsDone).get()); > > I think we should still show this in Web Inspector, as it’s useful for developers to know when debugging. You can show in the inspector side by comparing `index === -1`. Currently, inspector is not showing it.
Yusuke Suzuki
Comment 4 2020-01-12 15:02:07 PST
Created attachment 387492 [details] Patch
Keith Miller
Comment 5 2020-01-12 15:13:14 PST
Comment on attachment 387492 [details] Patch Have you run the microbenchmarks to see if there's a significant difference?
Yusuke Suzuki
Comment 6 2020-01-12 17:22:21 PST
(In reply to Keith Miller from comment #5) > Comment on attachment 387492 [details] > Patch > > Have you run the microbenchmarks to see if there's a significant difference? No difference. This change is basically for JSStringIterator to use the same mechanism to keep sizeof(JSStringIterator) small.
Keith Miller
Comment 7 2020-01-12 18:45:27 PST
Cool LGTM then
Yusuke Suzuki
Comment 8 2020-01-12 19:10:17 PST
Committed r254419: <https://trac.webkit.org/changeset/254419>
Radar WebKit Bug Importer
Comment 9 2020-01-12 19:11:15 PST
<rdar://problem/58517697>
Note You need to log in before you can comment on or make changes to this bug.