Closed
Bug 518730
Opened 15 years ago
Closed 15 years ago
TM: change the allocators to calloc
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
People
(Reporter: graydon, Assigned: graydon)
Details
(Whiteboard: fixed-in-tracemonkey)
Attachments
(1 file)
(deleted),
patch
|
gal
:
review+
|
Details | Diff | Splinter Review |
Calloc is faster on many platforms, it seems, than malloc + explicit-zero. I had to add a bit of explicit operator-new calloc nonsense get bug 516620 to be performance-neutral. Of course a more direct approach is to allocator-ize the objects in question and make the allocators calloc their internal chunks.
This has the advantage of letting us remove a few memsets *and* the offensive hunk in nanojit, which I didn't really want to have to argue Adobe into merging anyway.
Both my machines say this is a very minor performance win on sunspider.
Attachment #402723 -
Flags: review?(gal)
Updated•15 years ago
|
Attachment #402723 -
Flags: review?(gal) → review+
Assignee | ||
Comment 1•15 years ago
|
||
Whiteboard: fixed-in-tracemonkey
Comment 2•15 years ago
|
||
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Comment 3•15 years ago
|
||
> Calloc is faster on many platforms, it seems, than malloc +
> explicit-zero.
Graydon, I don't doubt your numbers. But I must be honest and say I'm
not convinced this is a good idea, for two reasons:
* It makes it impossible to use Valgrind/Memcheck to find places where
we should be initialising stuff, but we're not. Viz, it undermines
automatic verification of correctness.
* I wonder if this reduces performance on more modest hardware, for
example ARM, which might not have such fancy cache prefetch
algorithms and certainly has smaller caches. At least from a
simplistic interpretation of cache behaviour, callocing the entire
allocation area right at the start is going to cause a bunch of
cache misses which are not strictly necessary, particularly in the
case where only a small part of the allocation area is subsequently
used.
I will try to quantify the second point in the coming weeks.
Assignee | ||
Comment 4•15 years ago
|
||
No disagreement here. It spooked me out to observe, annoyed me to commit. I was just trying to get a patch in that appeared -- purely by virtue of changing from calloc to malloc + zero-set -- to have a minor regression.
Personally I'd choose to take the hit, but I couldn't find any other way to meet the no-regression rule on that patch. If you (or anyone) can find such a way, I'm happy to un-calloc this.
(I haven't actually been able to find a clear explanation of why it was even observable; after meandering around glibc for a while, I don't find an obvious code path showing that it should be. Though the code paths that exist seem all very specialized and it's hard to work out which one is running on any given machine.)
Comment 5•15 years ago
|
||
(In reply to comment #4)
>
> (I haven't actually been able to find a clear explanation of why it was even
> observable; after meandering around glibc for a while, I don't find an obvious
> code path showing that it should be. Though the code paths that exist seem all
> very specialized and it's hard to work out which one is running on any given
> machine.)
Yeah, glibc is horrible like that.
Assignee | ||
Updated•15 years ago
|
Flags: in-testsuite-
You need to log in
before you can comment on or make changes to this bug.
Description
•