Bug 20981 - Store per-class info in StructureID
Summary: Store per-class info in StructureID
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: Maciej Stachowiak
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-09-21 17:13 PDT by Maciej Stachowiak
Modified: 2008-09-22 14:06 PDT (History)
0 users

See Also:


Attachments
work in progress (30.82 KB, patch)
2008-09-21 17:14 PDT, Maciej Stachowiak
no flags Details | Formatted Diff | Diff
add per-class TypeInfo to the StructureID (right now only used for JSType) (50.01 KB, patch)
2008-09-21 19:41 PDT, Maciej Stachowiak
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Maciej Stachowiak 2008-09-21 17:13:27 PDT
Info that varies per JSCell class, currently often handled as virtual methods, should be stored in the StructureID.
Comment 1 Maciej Stachowiak 2008-09-21 17:14:12 PDT
Created attachment 23638 [details]
work in progress
Comment 2 Maciej Stachowiak 2008-09-21 19:41:00 PDT
Created attachment 23639 [details]
add per-class TypeInfo to the StructureID (right now only used for JSType)
Comment 3 Darin Adler 2008-09-21 19:51:36 PDT
Comment on attachment 23639 [details]
add per-class TypeInfo to the StructureID (right now only used for JSType)

I see this:

    OBJECT_OFFSET(StructureID, m_typeInfo) + OBJECT_OFFSET(TypeInfo, m_type)

Sometimes I've written that as:

    OBJECT_OFFSET(StructureID, m_typeInfo.m_type)

Not sure the pros and cons.

+#include "JSCallbackConstructor.h"
+#include "JSCallbackFunction.h"
+#include "JSCallbackObject.h"

What's this change to AllInOneFile.cpp about? Doesn't seem right to me. Maybe you pasted here by accident meaning to paste into JSGlobalObject.cpp at one point?

-        return m_structureID->type() == ObjectType;
+        return m_structureID->typeInfo().type() == ObjectType;

You could have sugared this by retaining a StructureID;:type() function. Why did you chose not to?

+#include "JSCallbackConstructor.h"
+#include "JSCallbackFunction.h"
+#include "JSCallbackObject.h"
 #include "ArrayConstructor.h"

Why aren't these sorted alphabetically in JSGlobalObject.cpp?

+++ JavaScriptCore/kjs/RegExpMatchesArray.h	(revision 0)
@@ -0,0 +1,47 @@
+/*
+ *  Copyright (C) 1999-2000 Harri Porten (porten@kde.org)
+ *  Copyright (C) 2003, 2007, 2008 Apple Inc. All Rights Reserved.

This class is new this year and doesn't need the pre-2008 copyrights on its source file.

+        TypeInfo(JSType type) : m_type(type) {}

We normally put spaces in those braces.

+    JSC::StructureID* createDOMStructure(JSC::ExecState*, CreateStructureIDFunction, const JSC::ClassInfo*, JSC::JSObject* prototype);

It's not helpful to pass the function for creating the structure instead of calling it. There's really no reason to do it that way. This should take a PassRefPtr<StructureID> instead, not take a prototype at all, and be renamed to cacheDOMStructure.

r=me
Comment 4 Maciej Stachowiak 2008-09-22 04:18:40 PDT
Landed this with most suggestions applied.
Comment 5 Darin Adler 2008-09-22 14:06:30 PDT
http://trac.webkit.org/changeset/36778