Last Comment Bug 710883 - Create js::BufferUnderloaded for use in JS engine
: Create js::BufferUnderloaded for use in JS engine
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
-- normal (vote)
: ---
Assigned To: Nicholas Nethercote [:njn]
: Jason Orendorff [:jorendorff]
Depends on: 708873 715453
  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:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Description User image 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 User image Nicholas Nethercote [:njn] 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 User image Luke Wagner [:luke] 2011-12-14 16:33:13 PST
How fast is moz_malloc_usable_size?
Comment 3 User image Nicholas Nethercote [:njn] 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 User image Luke Wagner [:luke] 2011-12-14 16:53:08 PST
Does realloc() always take a global lock?
Comment 5 User image Nicholas Nethercote [:njn] 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 User image 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 User image Luke Wagner [:luke] 2011-12-20 14:37:25 PST
P3 or just WONTFIX?
Comment 8 User image Nicholas Nethercote [:njn] 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 User image Nicholas Nethercote [:njn] 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 User image Nicholas Nethercote [:njn] 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.