Bug 88378 - Remove JSObject::m_inheritorID
Summary: Remove JSObject::m_inheritorID
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Hahnenberg
Depends on: 92338
  Show dependency treegraph
Reported: 2012-06-05 17:12 PDT by Gavin Barraclough
Modified: 2012-07-25 23:34 PDT (History)
6 users (show)

See Also:

Fix (9.52 KB, patch)
2012-06-05 17:16 PDT, Gavin Barraclough
no flags Details | Formatted Diff | Diff
Patch (9.27 KB, patch)
2012-07-25 17:05 PDT, Mark Hahnenberg
fpizlo: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Gavin Barraclough 2012-06-05 17:12:51 PDT
This is rarely used, and not performance critical (the commonly accessed copy is cached on JSFunction), and most objects don't need an inheritorID (this value is only used if the object is used as a prototype). Instead use a private named value in the object's property storage.
Comment 1 Gavin Barraclough 2012-06-05 17:16:36 PDT
Created attachment 145900 [details]
Comment 2 Geoffrey Garen 2012-06-05 17:19:28 PDT
Comment on attachment 145900 [details]

Comment 3 Gavin Barraclough 2012-06-05 22:08:55 PDT
Fixed in r119556
Comment 4 mitz 2012-06-06 00:33:31 PDT
(In reply to comment #3)
> Fixed in r119556

This broke the 32-bit build, as far as I can tell because it made the m_inlineStorage data member in JSNonFinalObject not 4-byte aligned, causing the assertion in finishCreation() to fail.
Comment 5 mitz 2012-06-06 00:37:33 PDT
I didn’t know how to fix the build, so I reverted this change in <http://trac.webkit.org/r119568>.
Comment 6 mitz 2012-06-06 00:38:09 PDT
Comment on attachment 145900 [details]

Changing to r- because it breaks the 32-bit build.
Comment 7 Gavin Barraclough 2012-06-07 21:29:58 PDT
Re-landed in r119795
Comment 8 Zoltan Arvai 2012-06-08 01:22:44 PDT
(In reply to comment #7)
> Re-landed in r119795

Layout test fails on Qt, GTK, EFL and Lion after r119795:
Comment 9 Gavin Barraclough 2012-06-08 01:55:23 PDT
Ugh.  I'll revert in the morning if no-one else has done so, & debug.

cheers for reporting this.
Comment 10 Gavin Barraclough 2012-06-08 14:31:41 PDT
Rolled back out in 119865. :'-(
Comment 11 Mark Hahnenberg 2012-07-25 17:05:24 PDT
Created attachment 154496 [details]
Comment 12 Mark Hahnenberg 2012-07-25 17:13:23 PDT
Committed r123682: <http://trac.webkit.org/changeset/123682>
Comment 13 Csaba Osztrogonác 2012-07-25 23:26:49 PDT
(In reply to comment #12)
> Committed r123682: <http://trac.webkit.org/changeset/123682>

It made many tests crash on 32 bit platforms. Could you check it, please?
new bug report for this regression - https://bugs.webkit.org/show_bug.cgi?id=92338
Comment 14 Csaba Osztrogonác 2012-07-25 23:34:54 PDT
Fixed by http://trac.webkit.org/changeset/123708. Thanks.