Bug 156732

Summary: Mark more classes as WTF_MAKE_FAST_ALLOCATED
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: WebCore Misc.Assignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, commit-queue, darin, kling
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

Description Chris Dumez 2016-04-18 22:30:24 PDT
Mark more classes as WTF_MAKE_FAST_ALLOCATED.
Comment 1 Chris Dumez 2016-04-18 22:32:04 PDT
Created attachment 276695 [details]
Patch
Comment 2 Chris Dumez 2016-04-19 09:03:14 PDT
Created attachment 276727 [details]
Patch
Comment 3 Darin Adler 2016-04-19 09:13:21 PDT
Comment on attachment 276727 [details]
Patch

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

Are these all classes that are actually allocated on the heap with new? The idea that we heap-allocate FloatQuad, FloatRoundedRect, and IntSize, for example, surprise me.

Do we have some good system for catching when we forget to do one of these? In the past I had wanted to come up with some trick with global allocator new to catch in the debugger if we miss one of these.

> Source/WebCore/dom/ActiveDOMCallbackMicrotask.h:39
> -    virtual ~ActiveDOMCallbackMicrotask();
> +    WEBCORE_EXPORT virtual ~ActiveDOMCallbackMicrotask();

Why this change?

> Source/WebCore/dom/ScriptExecutionContext.cpp:60
>  class ScriptExecutionContext::PendingException {
> -    WTF_MAKE_NONCOPYABLE(PendingException);
> +    WTF_MAKE_NONCOPYABLE(PendingException); WTF_MAKE_FAST_ALLOCATED;
>  public:

Looks like this class should be a struct.

> Source/WebCore/page/AutoscrollController.h:52
>  // AutscrollController handels autoscroll and pan scroll for EventHandler.

"handels"
Comment 4 Chris Dumez 2016-04-19 09:18:28 PDT
Comment on attachment 276727 [details]
Patch

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

>> Source/WebCore/dom/ActiveDOMCallbackMicrotask.h:39
>> +    WEBCORE_EXPORT virtual ~ActiveDOMCallbackMicrotask();
> 
> Why this change?

Trying to make Win-EWS happy:
     Creating library C:/cygwin/home/buildbot/WebKit/WebKitBuild/Release/lib32/DumpRenderTreeLib.lib and object C:/cygwin/home/buildbot/WebKit/WebKitBuild/Release/lib32/DumpRenderTreeLib.exp
WebCoreTestSupport.lib(Internals.obj) : error LNK2019: unresolved external symbol "public: virtual __thiscall WebCore::ActiveDOMCallbackMicrotask::~ActiveDOMCallbackMicrotask(void)" (??1ActiveDOMCallbackMicrotask@WebCore@@UAE@XZ) referenced in function "public: virtual void * __thiscall WebCore::ActiveDOMCallbackMicrotask::`scalar deleting destructor'(unsigned int)" (??_GActiveDOMCallbackMicrotask@WebCore@@UAEPAXI@Z) [C:\cygwin\home\buildbot\WebKit\WebKitBuild\Release\Tools\DumpRenderTree\DumpRenderTreeLib.vcxproj]
C:\cygwin\home\buildbot\WebKit\WebKitBuild\Release\bin32\DumpRenderTreeLib.dll : fatal error LNK1120: 1 unresolved externals

>> Source/WebCore/dom/ScriptExecutionContext.cpp:60
>>  public:
> 
> Looks like this class should be a struct.

OK

>> Source/WebCore/page/AutoscrollController.h:52
>>  // AutscrollController handels autoscroll and pan scroll for EventHandler.
> 
> "handels"

OK
Comment 5 Chris Dumez 2016-04-19 09:20:31 PDT
Comment on attachment 276727 [details]
Patch

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

>>> Source/WebCore/dom/ActiveDOMCallbackMicrotask.h:39
>>> +    WEBCORE_EXPORT virtual ~ActiveDOMCallbackMicrotask();
>> 
>> Why this change?
> 
> Trying to make Win-EWS happy:
>      Creating library C:/cygwin/home/buildbot/WebKit/WebKitBuild/Release/lib32/DumpRenderTreeLib.lib and object C:/cygwin/home/buildbot/WebKit/WebKitBuild/Release/lib32/DumpRenderTreeLib.exp
> WebCoreTestSupport.lib(Internals.obj) : error LNK2019: unresolved external symbol "public: virtual __thiscall WebCore::ActiveDOMCallbackMicrotask::~ActiveDOMCallbackMicrotask(void)" (??1ActiveDOMCallbackMicrotask@WebCore@@UAE@XZ) referenced in function "public: virtual void * __thiscall WebCore::ActiveDOMCallbackMicrotask::`scalar deleting destructor'(unsigned int)" (??_GActiveDOMCallbackMicrotask@WebCore@@UAEPAXI@Z) [C:\cygwin\home\buildbot\WebKit\WebKitBuild\Release\Tools\DumpRenderTree\DumpRenderTreeLib.vcxproj]
> C:\cygwin\home\buildbot\WebKit\WebKitBuild\Release\bin32\DumpRenderTreeLib.dll : fatal error LNK1120: 1 unresolved externals

I assume this is because I finalized the class and the destructor now gets called directly at once of the call sites.
Comment 6 Chris Dumez 2016-04-19 09:20:47 PDT
Created attachment 276728 [details]
Patch
Comment 7 Chris Dumez 2016-04-19 09:36:14 PDT
Created attachment 276731 [details]
Patch
Comment 8 Chris Dumez 2016-04-19 11:38:51 PDT
Comment on attachment 276731 [details]
Patch

Clearing flags on attachment: 276731

Committed r199735: <http://trac.webkit.org/changeset/199735>
Comment 9 Chris Dumez 2016-04-19 11:38:58 PDT
All reviewed patches have been landed.  Closing bug.