Bug 95737 – Extend the coverage of the Custom Allocation Framework in WTF and in JavaScriptCore

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Zoltan Horvath]

Add WTF_MAKE_FAST_ALLOCATED macro to some class declarations in WTF and in JavaScriptCore because these are instantiated by 'new'.

[Zoltan Horvath]

Created attachment 162020 [details]
proposed patch

[Eric Seidel (no email)]

What happens w/o this change?  Are there ports whcih would end up using sytem malloc for these objects by mistake?

[Zoltan Horvath]

(In reply to comment #2)
> What happens w/o this change?  Are there ports whcih would end up using sytem malloc for these objects by mistake?

Ports that don't customize ::new (e.g. qt) use system malloc for these allocations. The macro makes it straightforward for every port.

[Eric Seidel (no email)]

ok

[Benjamin Poulain]

May I suggest to extend the style checker to cover that?

Chances are people will forget it again. I know I am guilty of this.

[Zoltan Horvath]

(In reply to comment #5)
> May I suggest to extend the style checker to cover that?
> 
> Chances are people will forget it again. I know I am guilty of this.

It would be good, but we couldn't do that, since static source code analysis is required to find the places.

For example some JSValue classes don't have allocation operators, but stored on the JSGC's heap with placement new, or e.g. we don't want to customize memory allocation for QObjects.

I'm working on the WebCore's coverage now. I think it's not critical and it's okay if I check it sometimes.

[WebKit Review Bot]

Comment on attachment 162020 [details]
proposed patch

Clearing flags on attachment: 162020

Committed r127484: <http://trac.webkit.org/changeset/127484>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Zoltan Horvath]

(In reply to comment #6)

> For example some JSValue classes don't have allocation operators, but stored on the JSGC's heap with placement new

Let me correct myself. So it can happen that you customize the operator new of a class and you should have not done it. (e.g. basically it inherits the operator new from its base class what you can't see with a grep)

[Benjamin Poulain]

(In reply to comment #9)
> Let me correct myself. So it can happen that you customize the operator new of a class and you should have not done it. (e.g. basically it inherits the operator new from its base class what you can't see with a grep)

It does not have to be perfect.
You'd be surprised how dumb are some test in webkitpy, and yet they work 99% of the time :)

[Zoltan Horvath]

(In reply to comment #10)
> (In reply to comment #9)
> > Let me correct myself. So it can happen that you customize the operator new of a class and you should have not done it. (e.g. basically it inherits the operator new from its base class what you can't see with a grep)
> 
> It does not have to be perfect.
> You'd be surprised how dumb are some test in webkitpy, and yet they work 99% of the time :)

I see your point, but the incorrect use of this macro can lead to random crashes. Of course if anyone wants to give it a try to implement a checker for this in check-webkit-style I'd gladly support him.