699395 - Modify zone_good_size to call je_malloc_usable_size_in_advance

Status: RESOLVED FIXED

(Core :: Memory Allocator, defect)

Description

[Justin Lebar (not reading bugmail)]

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 [inactive]]

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)]

I don't know if it gets called.  We don't call it, but the system might.

Comment 3

[Nicholas Nethercote [inactive]]

I smell a premature optimization.  Some numbers on how often it's called would be nice.

Comment 4

[Justin Lebar (not reading bugmail)]

> 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 [inactive]]

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)]

It gets called a fair bit.  Loading about:home, slashdot, google, gmail, I get 23741 calls.

Comment 7

[Justin Lebar (not reading bugmail)]

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)]

Attachment: Patch v1

Comment 9

[Nicholas Nethercote [inactive]]

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)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b23b73290e38

Comment 11

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/b23b73290e38

Comment 12

[u279076]

Is this something QA can verify?

Comment 13

[Nicholas Nethercote [inactive]]

(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.