Last Comment Bug 699395 - Modify zone_good_size to call je_malloc_usable_size_in_advance
: Modify zone_good_size to call je_malloc_usable_size_in_advance
Status: RESOLVED FIXED
[inbound][qa-]
:
Product: Core
Classification: Components
Component: Memory Allocator (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla10
Assigned To: Justin Lebar (not reading bugmail)
:
: Mike Hommey [:glandium]
Mentors:
Depends on: 678977
Blocks:
  Show dependency treegraph
 
Reported: 2011-11-03 07:23 PDT by Justin Lebar (not reading bugmail)
Modified: 2011-12-28 16:06 PST (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed


Attachments
Patch v1 (1.15 KB, patch)
2011-11-04 09:22 PDT, Justin Lebar (not reading bugmail)
n.nethercote: review+
Details | Diff | Splinter Review

Description Justin Lebar (not reading bugmail) 2011-11-03 07:23:10 PDT
Currently,

zone_good_size(size) {
  p = malloc(size)
  ret = malloc_usable_size(p)
  free(p)
  return ret
}

Instead, it should just call je_malloc_usable_size_in_advance from bug 678977.
Comment 1 Nicholas Nethercote [:njn] 2011-11-03 16:27:12 PDT
Does zone_good_size() ever get called?  I see it's only relevant for Mac but I couldn't untangle the spaghetti further.
Comment 2 Justin Lebar (not reading bugmail) 2011-11-03 16:40:24 PDT
I don't know if it gets called.  We don't call it, but the system might.
Comment 3 Nicholas Nethercote [:njn] 2011-11-03 17:52:00 PDT
I smell a premature optimization.  Some numbers on how often it's called would be nice.
Comment 4 Justin Lebar (not reading bugmail) 2011-11-03 22:19:18 PDT
> Some numbers on how often it's called would be nice.

Sure.  But it's a totally brain-dead implementation now (comes from our Mac port, not originally from jemalloc), and we have a smart function which does exactly the same thing...  This doesn't count as code cleanup?
Comment 5 Nicholas Nethercote [:njn] 2011-11-03 22:32:18 PDT
Oh, if it's not original jemalloc code that's different.  It'd still be nice to know how often it's called.
Comment 6 Justin Lebar (not reading bugmail) 2011-11-04 08:18:35 PDT
It gets called a fair bit.  Loading about:home, slashdot, google, gmail, I get 23741 calls.
Comment 7 Justin Lebar (not reading bugmail) 2011-11-04 09:20:21 PDT
I thought zone_good_size was part of *our* Mac port, but it appears upstream, so I was probably wrong.
Comment 8 Justin Lebar (not reading bugmail) 2011-11-04 09:22:12 PDT
Created attachment 572000 [details] [diff] [review]
Patch v1
Comment 9 Nicholas Nethercote [:njn] 2011-11-06 13:43:10 PST
Comment on attachment 572000 [details] [diff] [review]
Patch v1

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

Stealing the review!  Looks fine.
Comment 10 Justin Lebar (not reading bugmail) 2011-11-07 18:09:33 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/b23b73290e38
Comment 11 Ed Morley [:emorley] 2011-11-08 01:11:50 PST
https://hg.mozilla.org/mozilla-central/rev/b23b73290e38
Comment 12 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2011-12-28 14:27:17 PST
Is this something QA can verify?
Comment 13 Nicholas Nethercote [:njn] 2011-12-28 14:29:01 PST
(In reply to Anthony Hughes, Mozilla QA (irc: ashughes) from comment #12)
> Is this something QA can verify?

Probably not, unless you pull out a debugger and step through the code.  I wouldn't bother, myself.

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