Closed Bug 213943 Opened 21 years ago Closed 11 years ago
Tracking of overhead involved in accessing DOM objects
This is a bug tracking overhead involved in accessing various DOM objects. The testcase is just something that does a no-op access a few hundred thousand times.
On that testcase, we are interested in whatever goes on outside nsHTMLDivElement::SetAttribute
I tried out the FindMember optimization patch in bug 213943. Surprisingly I only saw a modest gain of 2 to 4 percent. I verified that it was getting cached. The method being called is the 26th element of the second interface. The first interface only had one member. I suspect that the call overhead is only a small portion of the time in this loop. Unfortunately my system isn't good at running perf tests so I can't confirm this. It could be that the the difference between looping to the 26th method isn't that expensive over the hashing, but I would think we'd see a decent win in that situation. So the patch does help a bit, but apparently isn't the main source of time spent.
David, 2-4% win on raw time on that testcase means a 4-8% reduction in the JS/XPConnect overhead (since such overhead is only 50% of the total time; the other 50% is security manager and actually setting the attribute). Doesn't sound too shabby at all.
Depends on: 213813
new, malloc, and free are at the top of the list again for this tests as well. new seems to be driven by two areas. One is a string conversion between JS and native. The other is the nsNodeInfoManager::GetNodeInfo, which if you follow it up you'll get to nsGenericHTMLElement::NormalizeAttrString. What I find odd is that we're getting an allocation for each call to add a entry to the hash table via PL_HashTableAdd. I don't know much about the implementation but that doesn't seem very efficient. Shouldn't the hash table be block allocating entries in a more efficient manner? Is there a better hash to use here? At a higher level, nsNodeInfoManager seems to add and remove the entries for each call in GetNodeInfo and RemoveNodeInfo. So again I'm not sure why the hash logic has to do allocations, I would expect it to remain a constant size. malloc is driven by duplicating the string in nsDOMClassINfo::GetClassName, which in turn is generated by the security check. I wonder if it might be interesting to cache the results from the security check.
Odd. We should malloc from the hashtable code only if the hashtable is overloaded, if I read the code correctly.... Brendan? dbaron? Any idea what's up there? In general, btw, I'm ignoring anything happening under SetAttr for purposes of this bug; that code is in sore need of modification in various ways and I'd rather profile it once said modifications have taken place. As for GetClassName, see bug 213946 comment 0 (that talks about GetClassDescription, but the latter calls the former).
> We should malloc from the hashtable code only if the hashtable is
> overloaded, if I read the code correctly....
dbradley said PL_HashTableAdd. Are you reading PL_DHashTableOperate instead?
plhash does have allocEntry / freeEntry callbacks that can be used to use an
arena, but you may be better off using pldhash instead.
Hmm... Good point. Didn't jst do some perf tests and decide that plhash is faster than pldhash here? Also, see bug 166013
plhash may well be faster (and use more memory, most likely). But you could probably make it faster still with custom allocators using an arena, since the performance seems to be an issue here. (See http://lxr.mozilla.org/seamonkey/source/tools/trace-malloc/tmreader.c for an example.)
Can we take some of the findings here into 1.8 ?
What findings, exactly? There are no findings in this bug that I can see. Please stop spamming bugs with comments like this, Markus. It's very disheartening to wake up to a huge stream of pointless bugmail like this morning. Your time could be better-spent analyzing the profile and trying to find ways to improve the code.
I was refering to comment #10 - this sounds like a promising suggestion. Maybe someone has already tried that. I am more a QA guy no coder, especially that area of code is the game of a view I think.
> I was refering to comment #10 - this sounds like a promising suggestion. It's a fair amount of work, and could help a tad. There are certainly bigger performance fish to fry (security manager, eg). > I am more a QA guy no coder That's not immutable. ;)
BZ, is this still a problem? I tested with today's nightly build and got Average (10runs) :120ms On IE8 I got Average (10runs) :4031ms And Google Chrome 8 I got Average (10runs) :156ms
This is a tracking bug... The testcase was just an example. That said, we're definitely much better on this overhead, and we have some more ideas for reducing it further. See bug 622298. So it's fine to resolve this one worskforme or whatever, probably.
Using the testcase, average of 10 runs: Nightly 27 - 75ms Chrome 29 - 200ms IE 10 - 642ms
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → WORKSFORME
Yeah, WebIDL bindings have totally killed this dead as initially filed. ;)
You need to log in before you can comment on or make changes to this bug.