Bug 151242

Summary:

[JSC] JSPropertyNameEnumerator could be destructorless.

Product:

WebKit

Reporter:

Andreas Kling <kling>

Component:

JavaScriptCore

Assignee:

Andreas Kling <kling>

Status:

REOPENED

Severity:

Normal

CC:

commit-queue, keith_miller, kling, mark.lam, msaboff, ossy, saam

Priority:

Keywords:

Performance

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

151285, 151593

Bug Blocks:

Attachments:

Description	Flags
Hack for EWS	none
Patch	none
Follow-up patch to fix 32-bit platforms	none
Patch for relanding	none

Andreas Kling

Reported 2015-11-12 19:07:05 PST

I think we could make JSPropertyNameEnumerator destructorless by allocating the m_propertyNames vector out of GC copied space instead. The "vector" never changes size after construction anyway.

Attachments
Hack for EWS (6.58 KB, patch) 2015-11-12 19:08 PST, Andreas Kling	no flags	Details Formatted Diff Diff
Patch (7.30 KB, patch) 2015-11-12 19:22 PST, Andreas Kling	no flags	Details Formatted Diff Diff
Follow-up patch to fix 32-bit platforms (4.26 KB, patch) 2015-11-13 11:23 PST, Andreas Kling	no flags	Details Formatted Diff Diff
Patch for relanding (7.92 KB, patch) 2015-11-17 13:13 PST, Andreas Kling	no flags	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Andreas Kling

Comment 1 2015-11-12 19:08:38 PST

Created attachment 265463 [details] Hack for EWS

Andreas Kling

Comment 2 2015-11-12 19:22:00 PST

Created attachment 265464 [details] Patch With missing CopyToken + ChangeLog.

Mark Lam

Comment 3 2015-11-12 21:53:36 PST

Comment on attachment 265464 [details] Patch r=me

WebKit Commit Bot

Comment 4 2015-11-13 07:59:28 PST

Comment on attachment 265464 [details] Patch Clearing flags on attachment: 265464 Committed r192416: <http://trac.webkit.org/changeset/192416>

WebKit Commit Bot

Comment 5 2015-11-13 07:59:32 PST

All reviewed patches have been landed. Closing bug.

Csaba Osztrogonác

Comment 6 2015-11-13 10:03:19 PST

(In reply to comment #4) > Comment on attachment 265464 [details] > Patch > > Clearing flags on attachment: 265464 > > Committed r192416: <http://trac.webkit.org/changeset/192416> It made API tests assert on the 32 bit Apple Mac bots: https://build.webkit.org/builders/Apple%20El%20Capitan%2032-bit%20JSC%20%28BuildAndTest%29

Csaba Osztrogonác

Comment 7 2015-11-13 10:05:57 PST

(In reply to comment #6) > (In reply to comment #4) > > Comment on attachment 265464 [details] > > Patch > > > > Clearing flags on attachment: 265464 > > > > Committed r192416: <http://trac.webkit.org/changeset/192416> > > It made API tests assert on the 32 bit Apple Mac bots: > https://build.webkit.org/builders/Apple%20El%20Capitan%2032- > bit%20JSC%20%28BuildAndTest%29 and ~40 JSC tests fail on Apple Windows and 32 bit ARM bots: - https://build.webkit.org/builders/Apple%20Win%207%20Release%20%28Tests%29/builds/55097 - https://build.webkit.org/builders/EFL%20Linux%20ARMv7%20Thumb2%20Release - https://build.webkit.org/builders/EFL%20Linux%20ARMv7%20Traditional%20Release

Andreas Kling

Comment 8 2015-11-13 10:45:51 PST

(In reply to comment #6) > (In reply to comment #4) > > Comment on attachment 265464 [details] > > Patch > > > > Clearing flags on attachment: 265464 > > > > Committed r192416: <http://trac.webkit.org/changeset/192416> > > It made API tests assert on the 32 bit Apple Mac bots: > https://build.webkit.org/builders/Apple%20El%20Capitan%2032- > bit%20JSC%20%28BuildAndTest%29 Aha! It looks like copied space allocations are expected to be 8-byte divisible on 32-bit platforms too, even though pointers are 4 bytes there. I will make a quick fix that rounds up allocation sizes to the nearest multiple of 8 (same idea as DirectArguments when it allocates out of copied space.) Thanks for letting me know about the issue!

Andreas Kling

Comment 9 2015-11-13 11:23:14 PST

Created attachment 265485 [details] Follow-up patch to fix 32-bit platforms

Mark Lam

Comment 10 2015-11-13 11:25:45 PST

Comment on attachment 265485 [details] Follow-up patch to fix 32-bit platforms r=me

Andreas Kling

Comment 11 2015-11-13 14:16:54 PST

Comment on attachment 265485 [details] Follow-up patch to fix 32-bit platforms Clearing flags on attachment: 265485 Committed r192443: <http://trac.webkit.org/changeset/192443>

Andreas Kling

Comment 12 2015-11-13 14:17:01 PST

All reviewed patches have been landed. Closing bug.

WebKit Commit Bot

Comment 13 2015-11-13 17:14:44 PST

Re-opened since this is blocked by bug 151285

Andreas Kling

Comment 14 2015-11-17 13:13:16 PST

Created attachment 265702 [details] Patch for relanding This had a subtle problem on 32-bit, where WriteBarrier<Unknown> is 64-bit and WriteBarrier<JSString> is 64-bit, and so you really can't go around reinterpret_casting one to the other!

Andreas Kling

Comment 15 2015-11-17 13:13:44 PST

(In reply to comment #14) > Created attachment 265702 [details] > Patch for relanding > > This had a subtle problem on 32-bit, where WriteBarrier<Unknown> is 64-bit > and WriteBarrier<JSString> is 64-bit, and so you really can't go around > reinterpret_casting one to the other! I meant to say that WriteBarrier<Unknown> is 64-bit, and WriteBarrier<JSString> is 32-bit. :)

WebKit Commit Bot

Comment 16 2015-11-17 14:13:45 PST

Comment on attachment 265702 [details] Patch for relanding Clearing flags on attachment: 265702 Committed r192536: <http://trac.webkit.org/changeset/192536>

WebKit Commit Bot

Comment 17 2015-11-17 14:13:48 PST

All reviewed patches have been landed. Closing bug.

WebKit Commit Bot

Comment 18 2015-11-24 13:35:14 PST

Re-opened since this is blocked by bug 151593

Note You need to log in before you can comment on or make changes to this bug.