Bug 21152 - Speedup static property get/put
Summary: Speedup static property get/put
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore JavaScript (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac All
: P2 Normal
Assignee: Sam Weinig
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-09-26 14:40 PDT by Sam Weinig
Modified: 2008-09-26 19:36 PDT (History)
1 user (show)

See Also:


Attachments
patch (204.77 KB, patch)
2008-09-26 14:45 PDT, Sam Weinig
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sam Weinig 2008-09-26 14:40:34 PDT
Patch forthcoming.
Comment 1 Sam Weinig 2008-09-26 14:45:07 PDT
Created attachment 23858 [details]
patch
Comment 2 Oliver Hunt 2008-09-26 15:21:03 PDT
Do you have any performance measurements?
Comment 3 Darin Adler 2008-09-26 15:29:00 PDT
Comment on attachment 23858 [details]
patch

+++ JavaScriptCore/kjs/PropertySlot.h	(working copy)

Please take out the forward declaration of the HashEntry struct.

I'd love to see the actual functions (getRightContext) inlined as much as possible. It seems a shame to pay an extra function call for these little "trampolines".

+         void setInput(const UString&);

These look indented wrong. What happened?

+    // FIXME: other than sharing code, there is no reason this get function can't be simpler.

I think that "other than sharing code" is unnecessarily vague here. You need to be more specific about what is being shared.

-JSValue* nonCachingStaticFunctionGetter(ExecState* exec, const Identifier& propertyName, const PropertySlot& slot)

I think you forgot to remove the declaration in the JSDOMBinding.h header.

+    if (!static_cast<JSDOMWindowBase*>(slot.slotBase())->allowsAccessFrom(exec))
+        return jsUndefined();
+    return static_cast<JSDOMWindowBase*>(slot.slotBase())->getListener(exec, mousemoveEvent);

These could all be implemented by a call to a single inline helper instead of repeating this code over and over again.

+void setJSDOMWindowBaseEvent(ExecState*, JSObject*, JSValue*)
+{
+}

These empty functions probably need a comment to explain why they're here but empty.

+        void setListener(JSC::ExecState*, const AtomicString& eventType, JSC::JSValue* function);
     private:

I'd prefer to always have blank lines before private. I've noticed others recently deleting them, but I want them visually grouped with whatever comes after the private, not the things before.

+static JSValue* jsEventTargetNodeOnAbort(ExecState*, const Identifier&, const PropertySlot&);

You could use JS_EVENT_LISTENER_FOR_EACH_LISTENER to avoid having to write these all out.

+            # FIXME: this casts to int to match our previous behavior which turned 0xFFFFFFFF in -1 for NodeFilter.SHOW_ALL

You probably mean "into" here. Also, is there a cleaner way to handle this?

+        push(@implContent, "    { \"$key\", @$specials[$i], (intptr_t)@$value1[$i], (intptr_t)@$value2[$i] },\n");

C++ casts instead?

+CONSOLE MESSAGE: line 1: Unsafe JavaScript attempt to access frame with URL http://localhost:8000/security/resources/cross-frame-iframe-for-put-test.html from frame with URL http://127.0.0.1:8000/security/cross-frame-access-put.html. Domains, protocols and ports must match.

What? Why no explanation in the ChangeLog.

r=me -- all comments above optional
Comment 4 Sam Weinig 2008-09-26 19:36:43 PDT
Fixed in r36977.