[Webkit-unassigned] [Bug 22393] Segfault when caching property accesses to primitive cells.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=22393





------- Comment #4 from barraclough at apple.com  2008-11-20 21:44 PDT -------
Comments from Darin's original review of the polymorphic prototype list caching
code:

> > +    if (baseValue->isObject() &&
> > +        slot.isCacheable() &&
> > +        !asCell(baseValue)->structure()->isDictionary() &&
> > +        slot.slotBase() == asCell(baseValue)->structure()->prototypeForLookup(callFrame)) {
> 
> We normally try to format these with the operators at the starts of lines
> instead of the ends -- I think it makes things read a little bit clearer.
> 
> Those two places where you call asCell, I think I'd prefer asObject, because
> you've already established it's an object, and it's possible that we might some
> day implement an operation like structure() slightly more efficiently for
> JSObject than a JSCell. OK, it's not a realistic possibility, but still I like
> types to be as specific as possible in case that helps with optimization later.
> 
> > +        JSCell* baseCell = asCell(baseValue);
> 
> Same thought here.
> 
> I also think that if the values are worth putting into local variables for the
> code, maybe it's worth using a nested if and using the local variables in the
> if conditional too.

This may all no longer be relevant, assuming caching of accesses to cell
primitives is fixed, since the check at the top of this can then be
!JSImmediate::isImmediate(baseValue).  Repasted here incase these comments
provide useful guidance in refactoring the code, post bug fix.

> > +        // Heavy access to a prototype is a good indication that it's not being
> > +        // used as a dictionary.
> 
> Great comment, but I don't understand how the single call here indicates "heavy
> use". You might want to word it slightly differently to make that clearer.

Not *strictly* related to this bug as such ;-) , but a comment from Darin in
this area of the code.


-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.