Restoring a class via Epicea didn't recompile a reference?
Hi - I think I had this issue a long while back, but I just hit it again (desperately trying to get back into Pharo again)⦠I was trying out Willow - and had a method like this: initializeContainerView | behaviorDetails scopeSelection | behaviorDetails := IdentifiedWebView forDivNamed: 'behavior-details' containing: WPPagerDutyUserDetailsView new applying: [ :div :constants | â¦. Where I had a class reference to: WPPagerDutyUserDetailsView, I deleted that class thinking I didnât need it and then spotted the above method referenced it and so I brought it back via Epicea. However when I ran my Willow Application, my web page was showing an error message âError, ObsoleteClassWPPagerDutyUserDetailsView does not implementâ¦.â - essentially it seemed like this method was referencing the class I had deleted and not the one I had restored. When I went back to this method and simply reserved it, everything worked properly. I recall Marcus (at the time) mentioning that the Opal compiler was supposed to detect these kinds of changes and rebind things properly - but it seems that this isnât the case - or potentially restoring something from Epicea subverts this? I can raise a bug if its genuine - but Iâm just musing on this at the moment? Tim
That should be âre-saved â it, the issue went away. Sent from my iPhone
On 25 Apr 2018, at 23:43, Tim Mackinnon <tim@testit.works> wrote:
Hi - I think I had this issue a long while back, but I just hit it again (desperately trying to get back into Pharo again)â¦
I was trying out Willow - and had a method like this:
initializeContainerView
| behaviorDetails scopeSelection |
behaviorDetails := IdentifiedWebView forDivNamed: 'behavior-details' containing: WPPagerDutyUserDetailsView new applying: [ :div :constants | â¦.
Where I had a class reference to: WPPagerDutyUserDetailsView, I deleted that class thinking I didnât need it and then spotted the above method referenced it and so I brought it back via Epicea.
However when I ran my Willow Application, my web page was showing an error message âError, ObsoleteClassWPPagerDutyUserDetailsView does not implementâ¦.â - essentially it seemed like this method was referencing the class I had deleted and not the one I had restored.
When I went back to this method and simply reserved it, everything worked properly.
I recall Marcus (at the time) mentioning that the Opal compiler was supposed to detect these kinds of changes and rebind things properly - but it seems that this isnât the case - or potentially restoring something from Epicea subverts this?
I can raise a bug if its genuine - but Iâm just musing on this at the moment?
Tim
Hi, It looks you got bit by this issue: https://pharo.fogbugz.com/f/cases/21519/RemoveFromSystem-does-not-work-well-... It's not particular to Epicea. There should be a better management of Undeclareds in general in all Pharo. On Thu, Apr 26, 2018 at 8:44 AM, Tim Mackinnon <tim@testit.works> wrote:
That should be âre-saved â it, the issue went away.
Sent from my iPhone
On 25 Apr 2018, at 23:43, Tim Mackinnon <tim@testit.works> wrote:
Hi - I think I had this issue a long while back, but I just hit it again (desperately trying to get back into Pharo again)â¦
I was trying out Willow - and had a method like this:
initializeContainerView
| behaviorDetails scopeSelection |
behaviorDetails := IdentifiedWebView forDivNamed: 'behavior-details' containing: *WPPagerDutyUserDetailsView* new applying: [ :div :constants | â¦.
Where I had a class reference to: *WPPagerDutyUserDetailsView, *I deleted that class thinking I didnât need it and then spotted the above method referenced it and so I brought it back via Epicea.
However when I ran my Willow Application, my web page was showing an error message âError, ObsoleteClassWPPagerDutyUserDetailsView does not implementâ¦.â - essentially it seemed like this method was referencing the class I had deleted and not the one I had restored.
When I went back to this method and simply reserved it, everything worked properly.
I recall Marcus (at the time) mentioning that the Opal compiler was supposed to detect these kinds of changes and rebind things properly - but it seems that this isnât the case - or potentially restoring something from Epicea subverts this?
I can raise a bug if its genuine - but Iâm just musing on this at the moment?
Tim
-- 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
Yep - its that one. Needs fixing as it had me scratching my head in the wrong place ⦠its a bit out of my league. But we are hoping to see if we can fix a few bugs at the next UK Smalltalk meeting. Tim
On 26 Apr 2018, at 08:28, Guillermo Polito <guillermopolito@gmail.com <mailto:guillermopolito@gmail.com>> wrote:
Hi,
It looks you got bit by this issue:
https://pharo.fogbugz.com/f/cases/21519/RemoveFromSystem-does-not-work-well-... <https://pharo.fogbugz.com/f/cases/21519/RemoveFromSystem-does-not-work-well-...>
It's not particular to Epicea. There should be a better management of Undeclareds in general in all Pharo.
On Thu, Apr 26, 2018 at 8:44 AM, Tim Mackinnon <tim@testit.works <mailto:tim@testit.works>> wrote: That should be âre-saved â it, the issue went away.
Sent from my iPhone
On 25 Apr 2018, at 23:43, Tim Mackinnon <tim@testit.works <mailto:tim@testit.works>> wrote:
Hi - I think I had this issue a long while back, but I just hit it again (desperately trying to get back into Pharo again)â¦
I was trying out Willow - and had a method like this:
initializeContainerView
| behaviorDetails scopeSelection |
behaviorDetails := IdentifiedWebView forDivNamed: 'behavior-details' containing: WPPagerDutyUserDetailsView new applying: [ :div :constants | â¦.
Where I had a class reference to: WPPagerDutyUserDetailsView, I deleted that class thinking I didnât need it and then spotted the above method referenced it and so I brought it back via Epicea.
However when I ran my Willow Application, my web page was showing an error message âError, ObsoleteClassWPPagerDutyUserDetailsView does not implementâ¦.â - essentially it seemed like this method was referencing the class I had deleted and not the one I had restored.
When I went back to this method and simply reserved it, everything worked properly.
I recall Marcus (at the time) mentioning that the Opal compiler was supposed to detect these kinds of changes and rebind things properly - but it seems that this isnât the case - or potentially restoring something from Epicea subverts this?
I can raise a bug if its genuine - but Iâm just musing on this at the moment?
Tim
--
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
On 26 Apr 2018, at 09:28, Guillermo Polito <guillermopolito@gmail.com> wrote:
Hi,
It looks you got bit by this issue:
https://pharo.fogbugz.com/f/cases/21519/RemoveFromSystem-does-not-work-well-...
It's not particular to Epicea. There should be a better management of Undeclareds in general in all Pharo.
It can be fixed by moving the binding to undeclared in #removeFromSystem: This is done by adding Undeclared declare: self name asSymbol from: Smalltalk globals. before the "forgetClass: self logged:" line. What we need to do in addition: Only do that when the class is referenced. #isUsedClass: does that, but it does integrate over all methods in the system. (I sometimes wonder if we should not late-bind accesses to globals, that is, just compile them as message sends to the environment). Marcus
Two points. 1. Maybe we should centralize the removal of a class in a component as the class installer. So we can have it all in one place, and if we need to override and do fancy stuff. 2. Yes to the messages to the environment, I am doing so to have multiple environments sharing the same methods. We can check how much does it affect the performance, because it opens a lot of different options to extend the language. Cheers, Pablo On Wed, May 2, 2018 at 2:09 PM, Marcus Denker <marcus.denker@inria.fr> wrote:
On 26 Apr 2018, at 09:28, Guillermo Polito <guillermopolito@gmail.com> wrote:
Hi,
It looks you got bit by this issue:
https://pharo.fogbugz.com/f/cases/21519/RemoveFromSystem- does-not-work-well-with-Undeclareds
It's not particular to Epicea. There should be a better management of Undeclareds in general in all Pharo.
It can be fixed by moving the binding to undeclared in #removeFromSystem:
This is done by adding
Undeclared declare: self name asSymbol from: Smalltalk globals.
before the "forgetClass: self logged:" line.
What we need to do in addition: Only do that when the class is referenced. #isUsedClass: does that, but it does integrate over all methods in the system.
(I sometimes wonder if we should not late-bind accesses to globals, that is, just compile them as message sends to the environment).
Marcus
-- Pablo Tesone. tesonep@gmail.com
Well, yes to both. Now, about the performance, first I'd like to have a benchmark suite that runs all the time, to at least understand if we lose some performance, when/why was it. Of course, this would have an impact depending on how often are globals read/written. But I'd like to know with more certainty. Also [SystemNavigation new isUsedClass: IceRepository] bench => '172.497 per second' So maybe we can pay the price of a couple of milliseconds every time we remove a class for the moment. I prefer a "bit slower" system that is more robust :) Also, how often do we remove classes? I don't think this is critical, and the robustness here pays for itself :) We can think about how to optimize it later. Guille On Wed, May 2, 2018 at 2:20 PM, tesonep@gmail.com <tesonep@gmail.com> wrote:
Two points.
1. Maybe we should centralize the removal of a class in a component as the class installer. So we can have it all in one place, and if we need to override and do fancy stuff. 2. Yes to the messages to the environment, I am doing so to have multiple environments sharing the same methods. We can check how much does it affect the performance, because it opens a lot of different options to extend the language.
Cheers, Pablo
On Wed, May 2, 2018 at 2:09 PM, Marcus Denker <marcus.denker@inria.fr> wrote:
On 26 Apr 2018, at 09:28, Guillermo Polito <guillermopolito@gmail.com> wrote:
Hi,
It looks you got bit by this issue:
https://pharo.fogbugz.com/f/cases/21519/RemoveFromSystem-doe s-not-work-well-with-Undeclareds
It's not particular to Epicea. There should be a better management of Undeclareds in general in all Pharo.
It can be fixed by moving the binding to undeclared in #removeFromSystem:
This is done by adding
Undeclared declare: self name asSymbol from: Smalltalk globals.
before the "forgetClass: self logged:" line.
What we need to do in addition: Only do that when the class is referenced. #isUsedClass: does that, but it does integrate over all methods in the system.
(I sometimes wonder if we should not late-bind accesses to globals, that is, just compile them as message sends to the environment).
Marcus
-- Pablo Tesone. tesonep@gmail.com
-- 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
On 2 May 2018, at 14:39, Guillermo Polito <guillermopolito@gmail.com> wrote:
Well, yes to both.
Now, about the performance, first I'd like to have a benchmark suite that runs all the time, to at least understand if we lose some performance, when/why was it. Of course, this would have an impact depending on how often are globals read/written. But I'd like to know with more certainty.
Also
[SystemNavigation new isUsedClass: IceRepository] bench => '172.497 per second'
Only if it finds the class fast⦠you need to search for a not used one to see the worst case.
So maybe we can pay the price of a couple of milliseconds every time we remove a class for the moment. I prefer a "bit slower" system that is more robust :) Also, how often do we remove classes? I don't think this is critical, and the robustness here pays for itself :) We can think about how to optimize it later.
Yes, we do not remove classes that often. Marcus
Marcus Denker-4 wrote
(I sometimes wonder if we should not late-bind accesses to globals, that is, just compile them as message sends to the environment).
That would be *great*. This is another one of those little lies where "everything is a message send" and "late binding everything" breaks down, like inlining boolean/undefined ops, that can confuse/shock newbies. It probably made sense given the tech constraints at the time, but it would be wonderful not only to increase the beauty and simplicity of the system, but for the possibilities it opens up! ----- Cheers, Sean -- Sent from: http://forum.world.st/Pharo-Smalltalk-Users-f1310670.html
On 2 May 2018, at 17:33, Sean P. DeNigris <sean@clipperadams.com> wrote:
Marcus Denker-4 wrote
(I sometimes wonder if we should not late-bind accesses to globals, that is, just compile them as message sends to the environment).
That would be *great*. This is another one of those little lies where "everything is a message send" and "late binding everything" breaks down, like inlining boolean/undefined ops, that can confuse/shock newbies. It probably made sense given the tech constraints at the time, but it would be wonderful not only to increase the beauty and simplicity of the system, but for the possibilities it opens up!
We should really push more in this direction⦠the problem is that is is actually not easy to convince especially the âdie hard Smalltalkersâ that a change like that is a good thing⦠Marcus
On 2 May 2018, at 14:09, Marcus Denker <marcus.denker@inria.fr> wrote:
On 26 Apr 2018, at 09:28, Guillermo Polito <guillermopolito@gmail.com> wrote:
Hi,
It looks you got bit by this issue:
https://pharo.fogbugz.com/f/cases/21519/RemoveFromSystem-does-not-work-well-...
It's not particular to Epicea. There should be a better management of Undeclareds in general in all Pharo.
It can be fixed by moving the binding to undeclared in #removeFromSystem:
This is done by adding
Undeclared declare: self name asSymbol from: Smalltalk globals.
before the "forgetClass: self logged:" line.
What we need to do in addition: Only do that when the class is referenced. #isUsedClass: does that, but it does integrate over all methods in the system.
I did a PR: https://github.com/pharo-project/pharo/pull/1281 <https://github.com/pharo-project/pharo/pull/1281> Marcus
On 3 May 2018, at 10:15, Marcus Denker <marcus.denker@inria.fr> wrote:
On 2 May 2018, at 14:09, Marcus Denker <marcus.denker@inria.fr> wrote:
On 26 Apr 2018, at 09:28, Guillermo Polito <guillermopolito@gmail.com> wrote:
Hi,
It looks you got bit by this issue:
https://pharo.fogbugz.com/f/cases/21519/RemoveFromSystem-does-not-work-well-...
It's not particular to Epicea. There should be a better management of Undeclareds in general in all Pharo.
It can be fixed by moving the binding to undeclared in #removeFromSystem:
This is done by adding
Undeclared declare: self name asSymbol from: Smalltalk globals.
before the "forgetClass: self logged:" line.
What we need to do in addition: Only do that when the class is referenced. #isUsedClass: does that, but it does integrate over all methods in the system.
I did a PR:
In the end I split the PR in two: #isReferenced first (already in) and now the one liner to put classes that are still referenced into Undeclared: https://github.com/pharo-project/pharo/pull/1374 This just adds self isReferenced ifTrue: [Undeclared declare: self name asSymbol from: Smalltalk globals]. And it seems to work⦠Marcus
participants (5)
-
Guillermo Polito -
Marcus Denker -
Sean P. DeNigris -
tesonep＠gmail.com -
Tim Mackinnon