[Vm-dev] Performance, Quality and Process [was Array new: SmallInteger maxVal]

[Eliot Miranda]

Hi All,
    I'm not happy with this fix and I'm not happy with the lack of process
behind it.  First there has been insufficient discussion of what the right
behaviour is.  Second, 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.  Third, there is no review of fixes; we
just put them out there.

I'm concerned about performance, code quality and a lack of process for
agreeing fixes.  But at the same time I don't want to institute a
bureaucracy or slow down the pace of development.  Do others share my
concerns?  What suggestions have you?

One problem here is that Cog will introduce a huge raft of changes to the VM
and to Slang, and so possibly the whole issue is moot.  We'll face the
issues as we try and integrate my Cog VM into the squeakvm trunk.  But it
might be worth thinking a little about the issues up front.

David, I know you're technical lead, and I'm not trying to depose or
undermine you.  But I do think we can benefit from discussion and review of
major changes.  Alas, my Cog work not being generaly available yet is going
to cause problems down the line.  I need to at least hurry up and get the
StackInterpreter released.

On Tue, Oct 20, 2009 at 7:39 PM, David T. Lewis <lewis at mail.msen.com> wrote:

>
> On Tue, Oct 06, 2009 at 06:58:30PM -0400, David T. Lewis wrote:
> >
> > On Tue, Oct 06, 2009 at 10:05:02PM +0200, Nicolas Cellier wrote:
> > >
> > > >From http://code.google.com/p/pharo/issues/detail?id=1282
> > > Is this known?
> >
> > Thanks, it's known now :)
> >
> > Also added to Mantis at http://bugs.squeak.org/view.php?id=7405.
> >
> > Petr: Yes this is definitely a cool bug.
>
> A fix for this is on Mantis 7405 (http://bugs.squeak.org/view.php?id=7405
> ).
>
> The VMMaker updates are in SqueakSource in VMMaker-dtl.142.
>
> A separate patch is needed for platforms/unix/vm/sqUnixMemory.c (see the
> Mantis report, patch also sent to Ian).
>
> I have not tested Windows or Mac OS, but the basic scenario now is that
> the VM will limit the size of memory increase requests to 31 bit integer
> so support code can check for overflows. The requests from the VM are
> guaranteed (I hope) be valid 31 bit integers.
>
> Allocation requests of > 950 MB seem to be possible, although I do not
> have enough memory on my PC to verify that it actually works.
>
> Limiting allocation requests to more reasonable limits would be the
> responsibility of the image.
>
> Dave
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.squeakfoundation.org/pipermail/vm-dev/attachments/20091021/6cefe844/attachment.htm