Modify zone_good_size to call je_malloc_usable_size_in_advance

RESOLVED FIXED in Firefox 10

Status

()

Core
Memory Allocator
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: Justin Lebar (not reading bugmail), Assigned: Justin Lebar (not reading bugmail))

Tracking

unspecified
mozilla10
Points:
---

Firefox Tracking Flags

(firefox10 fixed)

Details

(Whiteboard: [inbound][qa-])

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
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.
(Assignee)

Updated

6 years ago
Assignee: nobody → justin.lebar+bug
Does zone_good_size() ever get called?  I see it's only relevant for Mac but I couldn't untangle the spaghetti further.
(Assignee)

Comment 2

6 years ago
I don't know if it gets called.  We don't call it, but the system might.
I smell a premature optimization.  Some numbers on how often it's called would be nice.
(Assignee)

Comment 4

6 years ago
> 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?
Oh, if it's not original jemalloc code that's different.  It'd still be nice to know how often it's called.
(Assignee)

Comment 6

6 years ago
It gets called a fair bit.  Loading about:home, slashdot, google, gmail, I get 23741 calls.
(Assignee)

Comment 7

6 years ago
I thought zone_good_size was part of *our* Mac port, but it appears upstream, so I was probably wrong.
(Assignee)

Comment 8

6 years ago
Created attachment 572000 [details] [diff] [review]
Patch v1
Attachment #572000 - Flags: review?(khuey)
Comment on attachment 572000 [details] [diff] [review]
Patch v1

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

Stealing the review!  Looks fine.
Attachment #572000 - Flags: review?(khuey) → review+
(Assignee)

Comment 10

6 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/b23b73290e38
status-firefox10: --- → fixed
Whiteboard: [inbound]
https://hg.mozilla.org/mozilla-central/rev/b23b73290e38
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla10
Is this something QA can verify?
Whiteboard: [inbound] → [inbound][qa?]
(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.
Whiteboard: [inbound][qa?] → [inbound][qa-]
You need to log in before you can comment on or make changes to this bug.