(temporarily popping back from the meta discussion)
On Wed, Oct 21, 2009 at 10:25:31AM -0700, Eliot Miranda wrote:
<snip>
the fix David has written does lots of computation (shifts) to check a valid size request that could be pushed earlier at initialization time, which would allow e.g. a vmParameterAt:put: to modify the max allocation request size.
<snip>
I'm concerned about performance, code quality and a lack of process for agreeing fixes.
Regarding performance associated with the changes, I was not able to measure any loss of performance. Actually, my crude test showed a slight improvement, which I can only attribute to random variation in the results.
Here is an example of one of the informal tests that I tried:
block := [oc := OrderedCollection new. (1 to: 1000000) do: [:e | oc add: (Array new: (e \ 27) + 1)]].
"Stock VM:" Smalltalk garbageCollect. before := (1 to: 5) collect: [:e | Time millisecondsToRun: block] ==> #(21393 20582 21511 21101 20761)
"VM with my Array alloc changes:" Smalltalk garbageCollect. after := (1 to: 5) collect: [:e | Time millisecondsToRun: block] ==> #(21582 20737 20693 20691 20725)
slowdownDueToTheChanges := (after sum - before sum / before sum) asFloat ==> -0.008732961233246
I got similar results for allocating strings, very slightly faster after the changes. I was happy with "not slower" and left it at that.
Can anyone suggest a more suitable benchmark?
Also, I'm running on AMD 64 and I was only guessing that integer shift and test sign would be a good approach. It might be awful on some hardware, I don't know.
r.e. vmParameterAt:put: to modify max allocation request size -- good idea. The changes that I made are strictly intended to protect against a VM crash or object memory corruption, nothing more. But some mechanism to prevent people from making unreasonable memory requests is clearly also needed.
Dave