two, hopefully, small issues

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

two, hopefully, small issues

mmimica
MagmaWbClassBuilder >> migrateFromTarget:to: starts with a "self halt". I assume that is just some debug leftover. Am I right?

In MagmaSession class >> cleanUp occurence of "(PointerFinder pointersTo: each)" should be replaced with "each pointersTo", since PointerFinder is deprecated (I'm talking Pharo 1.3 here).

I made a patch.


--
Milan Mimica
http://sparklet.sf.net

_______________________________________________
Magma mailing list
[hidden email]
http://lists.squeakfoundation.org/mailman/listinfo/magma

Magma-Client-MilanMimica.606(cmm.605).mcd (102K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: two, hopefully, small issues

Chris Muller-3
Hi Milan.  Currently all of the Magma tests pass on Pharo 1.3, but
yes, there are several areas where Pharo or Magma will need to be
updated to be 100% on Pharo.  Be sure also to get the change-set patch
for Pharo of items I expect to eventually be adopted into the standard
image.

The halt is there on purpose because it is a long-standing bug with
Avi's original WriteBarrier that, when it occurs, will need to be
closely looked at at that point in the code.  It could only possibly
happen when changing the class-definition of a class with instances
behind WB, so it probably wouldn't happen in prod (presumably, your
tests would catch it anyway, right?  :).

For PointerFinder, I didn't look at your patch, but wouldn't you need
to put the correct code in the -Pharo platform package?  I'm fine to
move the PointerFinder reference to the -Squeak package if necessary.
People of Pharo, please let me know!

Also, for patches, please submit them directly to the Magma Inbox
(a.k.a., "Magma tester") project of Squeaksource.  It is public
writable.

Thanks,
  Chris


On Sun, Dec 11, 2011 at 9:51 AM, Milan Mimica <[hidden email]> wrote:

> MagmaWbClassBuilder >> migrateFromTarget:to: starts with a "self halt". I
> assume that is just some debug leftover. Am I right?
>
> In MagmaSession class >> cleanUp occurence of "(PointerFinder pointersTo:
> each)" should be replaced with "each pointersTo", since PointerFinder is
> deprecated (I'm talking Pharo 1.3 here).
>
> I made a patch.
>
>
> --
> Milan Mimica
> http://sparklet.sf.net
>
> _______________________________________________
> Magma mailing list
> [hidden email]
> http://lists.squeakfoundation.org/mailman/listinfo/magma
>
_______________________________________________
Magma mailing list
[hidden email]
http://lists.squeakfoundation.org/mailman/listinfo/magma
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: two, hopefully, small issues

mmimica
On 12 December 2011 17:10, Chris Muller <[hidden email]> wrote:
The halt is there on purpose because it is a long-standing bug with
Avi's original WriteBarrier that, when it occurs, will need to be
closely looked at at that point in the code.  It could only possibly
happen when changing the class-definition of a class with instances
behind WB, so it probably wouldn't happen in prod (presumably, your
tests would catch it anyway, right?  :).

It did happen in development, and I expect it to happen often, because I do change class definitions while instances of a class behind WB exists. That is exactly what I like in smalltalk - programming the system in runtime, and I am not going to stop doing that. To kill all the instances I would have to clean Seaside seasons and run GC. So it's not a minor thing for me.
So what do I do about it?
You say it's a long-standing bug with original Avi's code
- do you know what was the bug about?
- do you know if the problem exists with the new WB?
- maybe I can look into it, even though so far I didn't notice any problems, I just removed the "halt"
- I can just switch the WB off like it used to be in Magma 1.2


For PointerFinder, I didn't look at your patch, but wouldn't you need
to put the correct code in the -Pharo platform package?  I'm fine to
move the PointerFinder reference to the -Squeak package if necessary.
People of Pharo, please let me know!

I see. It's up to you. The above bothers me more. Just know that PointerFinder is deprecated and I guess is going to be removed.


--
Milan Mimica
http://sparklet.sf.net

_______________________________________________
Magma mailing list
[hidden email]
http://lists.squeakfoundation.org/mailman/listinfo/magma
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: two, hopefully, small issues

Chris Muller-4
>> The halt is there on purpose because it is a long-standing bug with
>> Avi's original WriteBarrier that, when it occurs, will need to be
>> closely looked at at that point in the code.  It could only possibly
>> happen when changing the class-definition of a class with instances
>> behind WB, so it probably wouldn't happen in prod (presumably, your
>> tests would catch it anyway, right?  :).
>
> It did happen in development, and I expect it to happen often, because I do
> change class definitions while instances of a class behind WB exists. That
> is exactly what I like in smalltalk - programming the system in runtime, and
> I am not going to stop doing that. To kill all the instances I would have to
> clean Seaside seasons and run GC. So it's not a minor thing for me.

Sheesh, c'mon Milan, you don't need to tell me that.  You would just
not use WriteBarrier in development if the problem affects you..  If
it doesn't, great.

> So what do I do about it?
> You say it's a long-standing bug with original Avi's code
> - do you know what was the bug about?
> - do you know if the problem exists with the new WB?
> - maybe I can look into it, even though so far I didn't notice any problems,
> I just removed the "halt"

I dug up my old correspondence with Avi and forwarded it to this list.
 Please have a look and if it is no longer a problem or you have a
solution, please post it to Magma Inbox and I'd be happy to integrate
it.

> - I can just switch the WB off like it used to be in Magma 1.2

Yes, absolutely.

> I see. It's up to you. The above bothers me more. Just know that
> PointerFinder is deprecated and I guess is going to be removed.

It's not up to me -- it's up to you.  I delivered enough Pharo patches
so that all Magma 1.3 tests pass in Pharo 1.3.  It's now time for the
Pharo community to demonstrate its interest in Magma by cleaning up
these fringe issues for themselves.  I gave Magma a new
platform-independent package structure to make this easier.

 - Chris
_______________________________________________
Magma mailing list
[hidden email]
http://lists.squeakfoundation.org/mailman/listinfo/magma
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Fwd: two, hopefully, small issues

mmimica


---------- Forwarded message ----------
From: Milan Mimica <[hidden email]>
Date: 13 December 2011 19:44
Subject: Re: two, hopefully, small issues
To: Chris Muller <[hidden email]>


On 12 December 2011 21:37, Chris Muller <[hidden email]> wrote:

You would just
not use WriteBarrier in development if the problem affects you.. 

Running one setup in development and another in production is not an option. I am not going to do that.
 
I dug up my old correspondence with Avi and forwarded it to this list.
 Please have a look and if it is no longer a problem or you have a
solution, please post it to Magma Inbox and I'd be happy to integrate
it.

I can't seem to reproduce the problem. There is a snipped you posted and you claim it raises a primitive failure, but it doesn't for me. That's good news. It doesn't surprise me though. The problem showed up more that 6 years ago, it was a different VM and everything. I am just going to use the WB feature and I'll let you know if something horrible happens.



--
Milan Mimica
http://sparklet.sf.net



--
Milan Mimica
http://sparklet.sf.net

_______________________________________________
Magma mailing list
[hidden email]
http://lists.squeakfoundation.org/mailman/listinfo/magma
Loading...