Last Comment Bug 710883 - Create js::BufferUnderloaded for use in JS engine
: Create js::BufferUnderloaded for use in JS engine
Status: RESOLVED FIXED
[MemShrink:P3]
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- normal (vote)
: ---
Assigned To: Nicholas Nethercote [:njn] (on vacation until July 11)
:
Mentors:
Depends on: 708873 715453
Blocks:
  Show dependency treegraph
 
Reported: 2011-12-14 14:00 PST by Chris Leary [:cdleary] (not checking bugmail)
Modified: 2012-01-14 00:34 PST (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments

Description Chris Leary [:cdleary] (not checking bugmail) 2011-12-14 14:00:45 PST
Nick, per our discussion in bug 708873 comment 6, it would be very helpful to have a function internal to the JS engine that gave a conservative approximation for undesirable buffer slop to avoid triggering the memory reporter assertion. This would give us a central heuristic to tune in order to avoid unnecessary reallocs, but still avoid the memory allocator assertion for all practical purposes.

As we noted in that bug, StringBuffer can create strings from numerous places in the engine with various lengths and load factors (thanks to |Vector::reserve| and the vector space doubling behavior). If you could write a heuristic that we could slot into places like |StringBuffer::extractWellSized| that would be quite useful, so we could just say:

    if (length > CharBuffer::sMaxInlineStorage
        && BufferUnderloaded(length * sizeof(jschar), capacity * sizeof(jschar))) {
        // ... realloc the buffer to size ...
    }
Comment 1 Nicholas Nethercote [:njn] (on vacation until July 11) 2011-12-14 16:17:08 PST
I can't see a way to do this without using moz_malloc_usable_size or equivalent, and that's not doable within the JS engine.  (I tried defining a JS-specific equivalent a while ago and hit all sorts of troubles, I can't remember exactly why now, but they were bad enough that I found a different way to solve the problem.)

The only way forward is can see is to create a JSAPI function that lets you register a moz_malloc_usable_size-like function with the engine, and use that if it's been registered.  The browser would register just after the JS engine starts up.
Comment 2 Luke Wagner [:luke] 2011-12-14 16:33:13 PST
How fast is moz_malloc_usable_size?
Comment 3 Nicholas Nethercote [:njn] (on vacation until July 11) 2011-12-14 16:48:08 PST
(In reply to Luke Wagner [:luke] from comment #2)
> How fast is moz_malloc_usable_size?

Not that fast... it's similar to realloc() when it shrinks, judging from jemalloc's code.

Do we have evidence that unconditional realloc'ing is too slow?  They'll always be shrinking reallocs, which is the fastest case.
Comment 4 Luke Wagner [:luke] 2011-12-14 16:53:08 PST
Does realloc() always take a global lock?
Comment 5 Nicholas Nethercote [:njn] (on vacation until July 11) 2011-12-14 19:24:30 PST
It appears not to for non-huge allocations.  You can check for yourself in memory/jemalloc/jemalloc.c.
Comment 6 Luke Wagner [:luke] 2011-12-15 08:49:02 PST
(In reply to Nicholas Nethercote [:njn] from comment #5)
> You can check for yourself in memory/jemalloc/jemalloc.c.

You can do that ?!! ;-)  The path does look reasonably fast (the non-moving-small-realloc path is surprisingly easy to follow).  Given that extractWellSized() only reallocs when length > sMaxInlineLength and, for large strings, the cost of the the lock should be amortized by the cost of building the actual string, I think you're right that realloc() makes sense.
Comment 7 Luke Wagner [:luke] 2011-12-20 14:37:25 PST
P3 or just WONTFIX?
Comment 8 Nicholas Nethercote [:njn] (on vacation until July 11) 2011-12-20 15:15:59 PST
We need to do something to create the right-sized strings to avoid the assertion failure in the memory reporters.
Comment 9 Nicholas Nethercote [:njn] (on vacation until July 11) 2011-12-22 18:06:13 PST
There's a simpler option.  If you pass computedSize=0 as the second argument to mallocSizeOf() the size checking isn't performed.  The downside of this is that on platforms where moz_malloc_usable_size always returns 0, the string won't be counted at all.  But such platforms are very unusual.

So we could just pass computedSize=0 into the two calls to mallocSizeOf() in JSString::charsHeapSize().
Comment 10 Nicholas Nethercote [:njn] (on vacation until July 11) 2012-01-06 21:04:15 PST
This assertion was just removed in bug 715453.

Note You need to log in before you can comment on or make changes to this bug.