Investigate not using a hash table for the bloat log

NEW
Unassigned

Status

()

Core
XPCOM
3 years ago
3 years ago

People

(Reporter: jrmuizel, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

3 years ago
Created attachment 8570064 [details]
bloat.h

We're seeing a tremendous amount of time being spent waiting on mutexes on OS X in the NS_LogCtor, NS_LogDtor, etc.. I believe we could avoid this by having a static member per class. We can then avoid taking a global hash table lock and use a per-entry lock.

I've attached a prototype for what this could look like.

You use it like this:

struct A {
        TRACK_BLOAT(A);
};
Created attachment 8570640 [details] [diff] [review]
ridiculous hack

I've started integrating something like this into our code. It compiles, at least, but the results seem off.

And I get these warnings during linking:

ld: warning: direct access in ___cxx_global_var_init20 to global weak symbol guard variable for NS_RefCntLogger<mozilla::Mutex>::mEntry means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.
Comment on attachment 8570640 [details] [diff] [review]
ridiculous hack

Any opinions? If you can tell me that my efforts are in vain, now would be a good time.

The purpose of these changes is to make it possible to access the bloat entry outside of LOCK_TRACELOG() / UNLOCK_TRACELOG(), with the hopes that a per-class lock suffers from less contention.

I know that there are lots of problems in this patch regarding inferring types of subclasses, and GenericRefCounted is completely wrong, and changing functions in an array called "FrozenFunctions" is probably not a good idea, ...
Attachment #8570640 - Flags: feedback?(mh+mozilla)
We might not need to do this after all. Bug 1137963 looks like a much more promising approach, with better results.
Attachment #8570640 - Flags: feedback?(mh+mozilla) → feedback?(nfroyd)
FWIW, I think bug 1138616 would be another easy way to massively reduce the lock overhead.  We're tracking every addref and release right now, but for TreeHerder we only care about the creation and destruction of each object.
See Also: → bug 1137963, bug 1138616
Comment on attachment 8570640 [details] [diff] [review]
ridiculous hack

Review of attachment 8570640 [details] [diff] [review]:
-----------------------------------------------------------------

Canceling feedback without looking at this too closely, since the other bug seems less invasive and more promising.
Attachment #8570640 - Flags: feedback?(nfroyd)
You need to log in before you can comment on or make changes to this bug.