[Pharo 7.0-dev] Build #176: 20505-The-special-objects-arra
Locked
[Pharo 7.0-dev] Build #176: 20505-The-special-objects-arra
 
|
Re: [Pharo 7.0-dev] Build #176: 20505-The-special-objects-arra
|
|
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, <[hidden email]> wrote: There is a new Pharo build available!
|
Re: [Pharo 7.0-dev] Build #176: 20505-The-special-objects-arra
|
|
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. 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. PS: Who is nobody ? Cheers On Tue, Oct 10, 2017 at 2:04 PM, Guillermo Polito <[hidden email]> wrote:
Clément Béra Pharo consortium engineer Bâtiment B 40, avenue Halley 59650 Villeneuve d'Ascq |
Re: [Pharo 7.0-dev] Build #176: 20505-The-special-objects-arra
|
|
In reply to this post by Guillermo Polito
I agree with you, it is too dirty solution. The next try: Thank you for the pointers. Cheers, -- Pavel 2017-10-10 14:04 GMT+02:00 Guillermo Polito <[hidden email]>:
|
Re: [Pharo 7.0-dev] Build #176: 20505-The-special-objects-arra
|
|
In reply to this post by Clément Béra
On Tue, Oct 10, 2017 at 2:46 PM, Clément Bera <[hidden email]> wrote:
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...
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?
|
| Free forum by Nabble | Edit this page |