VMPI_alloca is problematic for several reasons. One, the memory it allocates is treated as pointer-containing (it's on the stack, or in a root, depending on platform and how large the object is), yet for the most part the data stored in that memory is not pointer-containing. There's a significant risk of misidentification. Two, conservative scanning of a root is more expensive than no scanning (duh). Three, pinning of stack memory is /really/ expensive, and from the stack we even retain objects that are reached by interior pointers, further increasing scanning expense and false retention rates. Four, for large alloca requests we create root memory and this means creating root segments for it, which adds to the set of roots and basically increases the root scanning burden (all roots are scanned twice). Though it may not be substantially cheaper to allocate non-pointer-containing temp memory on the heap which is then discarded pretty quickly, there are significant indirect benefits from doing so. We should use VMPI_alloca more sparingly than now.

Another tweak that might be useful is to introduce a variant of VMPI_alloca that explicitly allocates non-pointer-containing memory.

FileClass is shell code and not a big deal, but the ESC test is affected by this: a buffer the size of the file is allocated by VMPI_alloca.

Candidates (perhaps without obvious fixes, and perhaps not obviously wrong): AvmCore.cpp: wchar *buffer = (wchar*) VMPI_alloca(this, _buffer, (len16+1)*sizeof(wchar)); wchar* swapped = (wchar*)VMPI_alloca(this, _swapped, sizeof(wchar)*(len)); MathUtils.cpp: char* buffer = (char*)VMPI_alloca(core, _buffer, kMinSizeForInt64_t_toString); char* tmp = (char*)VMPI_alloca(core, _tmp, kMinSizeForDouble_base2_toString); char* buffer = (char*)VMPI_alloca(core, _buffer, kMinSizeForDouble_base10_toString); eval-cogen-stmt.cpp: Lcase = (Label**)VMPI_alloca(compiler->context->core, _Lcase, sizeof(Label*) * ncases);

Related bug: https://bugzilla.mozilla.org/show_bug.cgi?id=551766

You're right - they're more or less the same. Wonder why my search did not find it? I'll consolidate / divide up tasks.

Comment on attachment 475476 [details] [diff] [review] Fix for FileClass If a valid BOM is found, the string is created and returned without calling gc->Free(). If the newString*() methods are guaranteed not to throw, then a fixedmem allocation could be used. just observing, not quite suggesting. R+ with those points considered, take action or not as you see fit.

(In reply to comment #6) > Comment on attachment 475476 [details] [diff] [review] > Fix for FileClass > > If a valid BOM is found, the string is created and returned without calling > gc->Free(). OK, willfix. > If the newString*() methods are guaranteed not to throw, then a fixedmem > allocation could be used. just observing, not quite suggesting. Pointer-free GC allocations are not, as a rule, more expensive than fixed allocations, and are properly cleaned up to boot in case of exceptions. So I'll probably leave that alone.

Comment on attachment 475476 [details] [diff] [review] Fix for FileClass tamarin-redux changeset: 5228:646b4ff57973 (with requested changes)

(Leaving this open to track the work item, I may do more here.)

So far as I can tell, all uses of VMPI_alloca in the Flash Player are for allocating space for avmplus::Atom arrays (for arguments to a call), so a non-pointer API would only benefit Tamarin.