On Tue, Oct 10, 2017 at 2:46 PM, Clément Bera <[email protected]> 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 < > [email protected]> 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, <[email protected]> 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-arr >>> ay-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%20p >>> ull%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 >> <http://www.cnrs.fr>* >> >> >> *Web:* *http://guillep.github.io* <http://guillep.github.io> >> >> *Phone: *+33 06 52 70 66 13 <+33%206%2052%2070%2066%2013> >> > > > > -- > Clément Béra > Pharo consortium engineer > https://clementbera.wordpress.com/ > Bâtiment B 40, avenue Halley 59650 > <https://maps.google.com/?q=40,+avenue+Halley+59650%C2%A0Villeneuve+d'Ascq&entry=gmail&source=g>Villeneuve > d > <https://maps.google.com/?q=40,+avenue+Halley+59650%C2%A0Villeneuve+d'Ascq&entry=gmail&source=g> > 'Ascq > <https://maps.google.com/?q=40,+avenue+Halley+59650%C2%A0Villeneuve+d'Ascq&entry=gmail&source=g> > -- 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 <http://www.cnrs.fr>* *Web:* *http://guillep.github.io* <http://guillep.github.io> *Phone: *+33 06 52 70 66 13
