Bug 156732 - Mark more classes as WTF_MAKE_FAST_ALLOCATED
Summary: Mark more classes as WTF_MAKE_FAST_ALLOCATED
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-04-18 22:30 PDT by Chris Dumez
Modified: 2016-04-19 11:38 PDT (History)
4 users (show)

See Also:


Attachments
Patch (26.08 KB, patch)
2016-04-18 22:32 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (26.17 KB, patch)
2016-04-19 09:03 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (26.30 KB, patch)
2016-04-19 09:20 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (26.88 KB, patch)
2016-04-19 09:36 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.