Bug 143031 – New map and set modification tests in r181922 fails

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Mark Lam]

** The following JSC stress test failures have been introduced:
	stress/modify-map-during-iteration.js.always-trigger-copy-phase
	stress/modify-map-during-iteration.js.default
	stress/modify-map-during-iteration.js.dfg-eager
	stress/modify-map-during-iteration.js.dfg-eager-no-cjit-validate
	stress/modify-map-during-iteration.js.no-cjit-validate-phases
	stress/modify-map-during-iteration.js.no-llint
	stress/modify-set-during-iteration.js.always-trigger-copy-phase
	stress/modify-set-during-iteration.js.default
	stress/modify-set-during-iteration.js.dfg-eager
	stress/modify-set-during-iteration.js.dfg-eager-no-cjit-validate
	stress/modify-set-during-iteration.js.no-cjit-validate-phases
	stress/modify-set-during-iteration.js.no-llint

[Mark Lam]

See https://build.webkit.org/builders/Apple%20Win%207%20Release%20%28Tests%29/builds/50610.

[Yusuke Suzuki]

I'm now preparing a patch to fix it simply.
https://bugs.webkit.org/show_bug.cgi?id=142696

[Yusuke Suzuki]

Created attachment 249377 [details]
Patch

[Yusuke Suzuki]

Comment on attachment 249377 [details]
Patch

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

> Source/JavaScriptCore/ChangeLog:53
> +        when adjusting m_index. This is a little bit ugly, but it eliminates heap stored MapDataPatch.

Alternative fix is iterating twice.
1st:
Calculating newEnd and packing to destination by destination[newEnd] = entry;
2nd:
Iterating in reverse order and decrementing m_index.

[Build Bot]

Comment on attachment 249377 [details]
Patch

Attachment 249377 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/6471842135015424

New failing tests:
fast/regions/auto-size/autoheight-two-pass-layout-complex-002.html

[Build Bot]

Created attachment 249381 [details]
Archive of layout-test-results from ews100 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-mavericks  Platform: Mac OS X 10.9.5

[Alexey Proskuryakov]

I don't think that it's Windows only, they fail on Mac too. 

The EWS failure is unrelated.

[Yusuke Suzuki]

Created attachment 249389 [details]
Patch

Just updated the title of ChangeLog since this is not limited in Windows environment

[Geoffrey Garen]

Oops! This will teach me to land a patch and then go home :(.

[Geoffrey Garen]

Comment on attachment 249389 [details]
Patch

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

> Source/JavaScriptCore/runtime/MapData.h:73
> +            if (m_oldIndexUsedDuringPacking <= index)

I think we can accomplish this without an extra pass to save our old index. I'll post another tweak to do this.

[Geoffrey Garen]

Created attachment 249417 [details]
Patch

[Geoffrey Garen]

Committed r181968: <http://trac.webkit.org/changeset/181968>

[Yusuke Suzuki]

Comment on attachment 249389 [details]
Patch

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

>> Source/JavaScriptCore/runtime/MapData.h:73
>> +            if (m_oldIndexUsedDuringPacking <= index)
> 
> I think we can accomplish this without an extra pass to save our old index. I'll post another tweak to do this.

Great! Your tweak is very reasonable. Thank you!