Bug 25275 - use AtomicString to speed up window and document interceptors
Summary: use AtomicString to speed up window and document interceptors
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Antony Sargent
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-04-17 16:02 PDT by Antony Sargent
Modified: 2009-04-30 10:28 PDT (History)
1 user (show)

See Also:


Attachments
patch v1 (3.01 KB, patch)
2009-04-17 16:06 PDT, Antony Sargent
eric: review-
Details | Formatted Diff | Diff
patch v2 (2.98 KB, patch)
2009-04-29 15:43 PDT, Antony Sargent
eric: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Antony Sargent 2009-04-17 16:02:47 PDT
Patch forthcoming.
Comment 1 Antony Sargent 2009-04-17 16:06:43 PDT
Created attachment 29590 [details]
patch v1
Comment 2 Dimitri Glazkov (Google) 2009-04-17 16:19:35 PDT
Comment on attachment 29590 [details]
patch v1

Looks good.

> +        http://codereview.chromium.org/67141

Don't need Chromium review in ChangeLog, probably. Just mention it on bug.
Comment 3 Eric Seidel (no email) 2009-04-29 14:48:45 PDT
Comment on attachment 29590 [details]
patch v1

WebKit uses DEFINE_STATIC_LOCAL for local statics.  The Mac build will have trouble if we don't here too.

// Use these to declare and define a static local variable (static T;) so that
//  it is leaked so that its destructors are not called at exit. Using this
//  macro also allows workarounds a compiler bug present in Apple's version of GCC 4.0.1.
#if COMPILER(GCC) && defined(__APPLE_CC__) && __GNUC__ == 4 && __GNUC_MINOR__ == 0 && __GNUC_PATCHLEVEL__ == 1
#define DEFINE_STATIC_LOCAL(type, name, arguments) \
    static type* name##Ptr = new type arguments; \
    type& name = *name##Ptr
#else
#define DEFINE_STATIC_LOCAL(type, name, arguments) \
    static type& name = *new type arguments
#endif
Comment 4 Eric Seidel (no email) 2009-04-29 14:49:09 PDT
cc-ing Dimitri so he can see my comments.
Comment 5 Antony Sargent 2009-04-29 15:43:47 PDT
Created attachment 29896 [details]
patch v2

Switched to using DEFINE_STATIC_LOCAL. Thanks for catching this Eric!
Comment 6 Eric Seidel (no email) 2009-04-29 15:44:54 PDT
Comment on attachment 29896 [details]
patch v2

Looks fine.  Will land.
Comment 7 Eric Seidel (no email) 2009-04-29 15:48:36 PDT
Do we have any performance tests which show that this is actually faster?

I'm going to land it as-is... but it seems this sort of change should have some perf-data associated with it.
Comment 8 Eric Seidel (no email) 2009-04-29 15:49:00 PDT
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebCore/ChangeLog
	M	WebCore/bindings/v8/custom/V8DOMWindowCustom.cpp
	M	WebCore/bindings/v8/custom/V8HTMLDocumentCustom.cpp
Committed r43022
Comment 9 Dimitri Glazkov (Google) 2009-04-30 09:48:59 PDT
Whoops. Last patch actually introduced variable redefinition. Fixing...
Comment 10 Dimitri Glazkov (Google) 2009-04-30 10:28:01 PDT
Fix landed in http://trac.webkit.org/changeset/43066.