On Tue, Oct 10, 2017 at 2:46 PM, Cl��ment Bera <bera.clement@gmail.com> wrote:
Hi Guille,

For "First":
I looked into this problem with Pavel this morning. The special object array being incorrectly created leads to problems when a large object (bigger that growthHeadroom) is created, as it does not go through the handleFailingBasicNew methods. Typically, Pavel could not load Seaside into the minimal image because of that. The problem lies with the initialization of error codes.

Yes we were discussing this with Pavel yesterday too. I pointed him to look for your blogpost on sunday when he would not load seaside :).

We I meant is that this should have been in a comment at least in the pull request. Otherwise your private discussion gets lost...
��

For "Second":
I don't mind what you do as long as it solves the problem of large allocations on minimal images. In any case the problems lies with the error code symbols, so that the code in basicNew:��
ec == #'insufficient object memory' ifTrue:
[^self handleFailingBasicNew: sizeRequested].
takes the correct branch. Before Pavel's change, the condition was false during bootstrap leading to large allocation crashing the VM, ec was equal to 9 instead of��#'insufficient object memory', since the error codes in the special object array were incorrectly initialized somehow.

Yes, I understand the error.

Now, I was crunching it a bit yesterday, and this looks too error prone. This has a super hidden dependency with the special objects array.

I ask myself: do we want this to fail when the special object's array changes? What if somebody suddenly tries to "fix a typo" in the error message and forgots to update the special object's array?

Moreover, Why not doing

ec == ((Smalltalk specialObjectsArray at: 53) at: 9) ifTrue:
[^self handleFailingBasicNew: sizeRequested].

And why not adding also a guard in case ec == 9?

ec == 9 ifTrue:
[^self handleFailingBasicNew: sizeRequested].
ec == ((Smalltalk specialObjectsArray at: 53) at: 9) ifTrue:
[^self handleFailingBasicNew: sizeRequested].

Like that the fallback code could be a bit more robust.

What do you think?
��

PS: Who is nobody ?

Cheers


On Tue, Oct 10, 2017 at 2:04 PM, Guillermo Polito <guillermopolito@gmail.com> wrote:
Hi Pavel,

I don't like this change.

First, there is no comment nor explanation why that is needed. This change is obscure, if we forget in a couple of months why that line of code is there we may��
��- delete it unintentionally
��- or leave it forever and fear to clean it up because we do not understand it :(

Second, It looks that recreating the special objects array there, like that, means that it was not correctly created from the beginning. We should instead create it correctly from the beginning.

I think we should revert this change.

On Tue, Oct 10, 2017 at 12:24 PM, <ci-pharo-ci-jenkins2@inria.fr> wrote:
There is a new Pharo build available!

The status of the build #176 was: SUCCESS.

The Pull Request #336 was integrated: "20505-The-special-objects-array-needs-to-be-recreated-during-bootstrap"
Pull request url: https://github.com/pharo-project/pharo/pull/336

Issue Url: https://pharo.fogbugz.com/f/cases/20505
Build Url: https://ci.inria.fr/pharo-ci-jenkins2/job/Test%20pending%20pull%20request%20and%20branch%20Pipeline/job/development/176/



--

������

Guille Polito

Research Engineer

Centre de Recherche en Informatique, Signal et Automatique de Lille

CRIStAL - UMR 9189

French National Center for Scientific Research - http://www.cnrs.fr


Web: http://guillep.github.io

Phone: +33 06 52 70 66 13




--
Cl��ment B��ra
Pharo consortium engineer



--

������

Guille Polito

Research Engineer

Centre de Recherche en Informatique, Signal et Automatique de Lille

CRIStAL - UMR 9189

French National Center for Scientific Research - http://www.cnrs.fr


Web: http://guillep.github.io

Phone: +33 06 52 70 66 13