Closed Bug 802243 Opened 12 years ago Closed 12 years ago

WrapperCache DOMStringMap

Categories

(Core :: DOM: Core & HTML, defect)

x86
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: smaug, Assigned: smaug)

References

Details

Attachments

(3 files, 2 obsolete files)

Attached patch WIP (obsolete) — Splinter Review
With this patch, the "dataset" case shows "270,904" on http://jsperf.com/data-dataset/2.

That's compared to "212,353" without the patch and "260,409" for Chrome.
Oh, and on http://jsperf.com/data-dataset we end up at "894,407" with patch compared to "689,389" without.
With this, I get "413,135" on http://jsperf.com/data-dataset/2 and "1,235,610" on http://jsperf.com/data-dataset

That's about as good as we can do until we have WebIDL for HTMLElement.   At that point I fully expect even better numbers.
Attached patch wrappercacheSplinter Review
Changed nsDOMStringMapSH::PreCreate to be more like other ::PreCreates
Attachment #671928 - Attachment is obsolete: true
Attachment #671963 - Flags: review?(bzbarsky)
Comment on attachment 671963 [details] [diff] [review]
wrappercache

The comment about nsIDOMMutationObserver should talk about nsIDOMDOMStringMap, right?

r=me with that.
Attachment #671963 - Flags: review?(bzbarsky) → review+
Attachment #671947 - Flags: review?(bugs)
Attached patch w2Splinter Review
Yes.
Comment on attachment 671947 [details] [diff] [review]
Patch on top of that that adds a custom quickstub

>+nsDOMStringMap*
>+nsGenericHTMLElement::Dataset()
> {
>   nsDOMSlots *slots = DOMSlots();
> 
>   if (!slots->mDataset) {
>     // mDataset is a weak reference so assignment will not AddRef.
>     // AddRef is called before assigning to out parameter.
>     slots->mDataset = new nsDOMStringMap(this);
>   }
> 
>-  NS_ADDREF(*aDataset = slots->mDataset);
>+  return slots->mDataset;
>+}
I think we want to make this to return already_AddRefed.
It is scary is slots->mDataset points to an object which has refcount 0.


>+nsGenericHTMLElement::GetDataset(nsIDOMDOMStringMap** aDataset)
>+{  
>+  NS_ADDREF(*aDataset = Dataset());
then this would be
*aDataset = Dataset().get();


>+    'nsIDOMHTMLElement_GetDataset': {
>+        'thisType' : 'nsGenericHTMLElement',
>+        'code': '    nsDOMStringMap *result = self->Dataset();',
And this could be then
nsRefPtr<nsDOMStringMap> result = self->Dataset();
Attachment #671947 - Flags: review?(bugs) → review+
Attachment #671947 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: