Every time a string is passed as a DOMString/AString from JS to C++ through XPConnect and the C++ code touches the string, the string code ends up having to allocate a buffer handle. The allocations (new's) show up when profiling DOM code, and these allocations accounts for ~450 of the mallocs we do when starting mozilla and immediately exiting. I bet these allocations happen quite frequently while running the browser too. Patch coming up that makes the XPCReadableJSStringWrapper code only allocate the buffer handle when a shared handle is requested, not when a non-shared handle is requested. This is done at the expense of increasing the size of the XPCReadableJSStringWrapper class, but instances of the XPCReadableJSStringWrapper class are always transient, they're never held in memory for a long time.

I believe this patch is ready for reviews, anyone wanto have a look?

Comment on attachment 65569 [details] [diff] [review] Don't allocate non-shared buffer handles. Looks good, sr=jag. I'd appreciate it if dbaron could be the r= since this builds on his code.

Comment on attachment 65569 [details] [diff] [review] Don't allocate non-shared buffer handles. I did something similar in my work on AtomString that I never finished, and I'd like to do something similar for nsXPIDL[C]String... mBufferHandle should just be an nsBufferHandle<PRUnichar> -- why use a 5-word handle when you can use a 2-word handle? Perhaps you could rename |GetVoidBufferHandle| to |GetSharedEmptyBufferHandle| for consistency with other string classes? (Unless you meant it to set the kIsNULL bit.) For |GetBufferHandle|, I see no reason that you need the |!mStr| check -- won't mBufferHandle always be valid? I think you'll also want to override GetFlatBufferHandle so that calling PromiseFlatString doesn't lead to the creation of a shared buffer handle -- it can just call GetBufferHandle.

Thanks dbaron, how about this version? I decided to get rid of GetVoidBufferHandle() and put the code inline in GetSharedBufferHandle() since that's the only place it's needed (as you suggested).

Comment on attachment 65678 [details] [diff] [review] Incorporate dbaron's suggestions r=dbaron, although I'm not sure why you want to change it from inheriting from ...WithAllocator to ...WithDestroy.

Because then I don't need an allocator, if there are clear benefits to using ...WithAllocator over ...WithDestroy then I could switch back, let me know if I should...

Comment on attachment 65678 [details] [diff] [review] Incorporate dbaron's suggestions sr=jag

Fixed.

Marking Verified -