Closed Bug 518730 Opened 15 years ago Closed 15 years ago

TM: change the allocators to calloc

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: graydon, Assigned: graydon)

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(1 file)

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)
Attachment #402723 - Flags: review?(gal) → review+
Whiteboard: fixed-in-tracemonkey
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
> 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.
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.)
(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.
Flags: in-testsuite-
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: