[Pharo-project] Cleaning up SmartRefStream use
Hi, We should minimize the use of SmartRefStream... so we can remove it eventually. I already did a little bit (e.g. removing BitmapStreamTests). These are the next steps: Issue 5081: Remve Morph saving to file/loading http://code.google.com/p/pharo/issues/detail?id=5081 Anyone needs this? Issue 5084: Remove textStyle FileOut http://code.google.com/p/pharo/issues/detail?id=5084 Is this used? Strange corner of the system... lots of dead code. Issue 5083: remove unused methods from SmartRefStream http://code.google.com/p/pharo/issues/detail?id=5083 Towards removing it alltogether. -- Marcus Denker -- http://marcusdenker.de
Indeed this is fun I was doing the same :) Now I do not understand why some parts of the system uses dataStream, others ReferenceStream and finally others SmartRefStream why not all of them use SmartRefStream. Stef On Dec 11, 2011, at 10:43 AM, Marcus Denker wrote:
Hi,
We should minimize the use of SmartRefStream... so we can remove it eventually.
I already did a little bit (e.g. removing BitmapStreamTests). These are the next steps:
Issue 5081: Remve Morph saving to file/loading http://code.google.com/p/pharo/issues/detail?id=5081
Anyone needs this?
Issue 5084: Remove textStyle FileOut http://code.google.com/p/pharo/issues/detail?id=5084
Is this used? Strange corner of the system... lots of dead code.
Issue 5083: remove unused methods from SmartRefStream http://code.google.com/p/pharo/issues/detail?id=5083
Towards removing it alltogether.
-- Marcus Denker -- http://marcusdenker.de
2011/12/11 Stéphane Ducasse <stephane.ducasse@inria.fr>:
Indeed this is fun I was doing the same :) Now I do not understand why some parts of the system uses dataStream, others ReferenceStream and finally others SmartRefStream why not all of them use SmartRefStream.
Stef
That's a very good question. Where should design decisions be documented? On one side, we should have requirements that we document with some high level TestCase. But whether the implementation is guided by time/memory efficiency, code simplicity, or just historically available packages is not easily described by tests. Obviously, you would have a hard time discovering this in legacy code (digging squeak wiki and mailing list?). The question should apply to Pharo too. Pharo is young, so maybe all design decisions can be retrieved by virtue of human memory... But will it be the case in 5 years? Nicolas
On Dec 11, 2011, at 10:43 AM, Marcus Denker wrote:
Hi,
We should minimize the use of SmartRefStream... so we can remove it eventually.
I already did a little bit (e.g. removing BitmapStreamTests). These are the next steps:
Issue 5081:  Remve Morph saving to file/loading    http://code.google.com/p/pharo/issues/detail?id=5081
   Anyone needs this?
Issue 5084:  Remove textStyle FileOut    http://code.google.com/p/pharo/issues/detail?id=5084
   Is this used? Strange corner of the system... lots of dead code.
Issue 5083:  remove unused methods from SmartRefStream    http://code.google.com/p/pharo/issues/detail?id=5083
   Towards removing it alltogether.
-- Marcus Denker -- http://marcusdenker.de
That's a very good question. Where should design decisions be documented? On one side, we should have requirements that we document with some high level TestCase. But whether the implementation is guided by time/memory efficiency, code simplicity, or just historically available packages is not easily described by tests. Obviously, you would have a hard time discovering this in legacy code (digging squeak wiki and mailing list?). The question should apply to Pharo too. Pharo is young, so maybe all design decisions can be retrieved by virtue of human memory... But will it be the case in 5 years?
We will introduce package manifesto and they should hold this. as well as false positive for rules checking and documentation. I think that any successful system will have such problem. Now high level rules, tests are documentation are important. Stef
On Sun, Dec 11, 2011 at 11:32 AM, Stéphane Ducasse < stephane.ducasse@inria.fr> wrote:
Indeed this is fun I was doing the same :) Now I do not understand why some parts of the system uses dataStream, others ReferenceStream and finally others SmartRefStream why not all of them use SmartRefStream.
It is not that I like these guys, but their class comments are pretty clear: - DataStream does NOT handle cycles, but hence it may be faster in certain scenarios. I guess that for example, Monticello is sure that in its definitions, there cannot be cycles. In addition, it doesn't take into account variables names/order and hence class reshape isn't supported. - ReferenceStream supports cycles but still doesn't support class reshape. - SmartRefStream supports cycles and class reshape.
Stef
On Dec 11, 2011, at 10:43 AM, Marcus Denker wrote:
Hi,
We should minimize the use of SmartRefStream... so we can remove it eventually.
I already did a little bit (e.g. removing BitmapStreamTests). These are the next steps:
Issue 5081: Remve Morph saving to file/loading http://code.google.com/p/pharo/issues/detail?id=5081
Anyone needs this?
Issue 5084: Remove textStyle FileOut http://code.google.com/p/pharo/issues/detail?id=5084
Is this used? Strange corner of the system... lots of dead code.
Issue 5083: remove unused methods from SmartRefStream http://code.google.com/p/pharo/issues/detail?id=5083
Towards removing it alltogether.
-- Marcus Denker -- http://marcusdenker.de
-- Mariano http://marianopeck.wordpress.com
On Dec 11, 2011, at 4:26 PM, Mariano Martinez Peck wrote:
On Sun, Dec 11, 2011 at 11:32 AM, Stéphane Ducasse <stephane.ducasse@inria.fr> wrote: Indeed this is fun I was doing the same :) Now I do not understand why some parts of the system uses dataStream, others ReferenceStream and finally others SmartRefStream why not all of them use SmartRefStream.
It is not that I like these guys, but their class comments are pretty clear: - DataStream does NOT handle cycles, but hence it may be faster in certain scenarios. I guess that for example, Monticello is sure that in its definitions, there cannot be cycles. In addition, it doesn't take into account variables names/order and hence class reshape isn't supported. - ReferenceStream supports cycles but still doesn't support class reshape. - SmartRefStream supports cycles and class reshape.
sounds good then we should be able to replace all the reference stream and smart by fuel. stef
On Sun, Dec 11, 2011 at 4:35 PM, Stéphane Ducasse <stephane.ducasse@inria.fr
wrote:
On Dec 11, 2011, at 4:26 PM, Mariano Martinez Peck wrote:
On Sun, Dec 11, 2011 at 11:32 AM, Stéphane Ducasse <
stephane.ducasse@inria.fr> wrote:
Indeed this is fun I was doing the same :) Now I do not understand why some parts of the system uses dataStream, others ReferenceStream and finally others SmartRefStream why not all of them use SmartRefStream.
It is not that I like these guys, but their class comments are pretty clear: - DataStream does NOT handle cycles, but hence it may be faster in certain scenarios. I guess that for example, Monticello is sure that in its definitions, there cannot be cycles. In addition, it doesn't take into account variables names/order and hence class reshape isn't supported. - ReferenceStream supports cycles but still doesn't support class reshape. - SmartRefStream supports cycles and class reshape.
sounds good
Yes it does. In fact, I would love also to have those things optional/configurable in Fuel: optional cycle detection and optional class reshape support :)
then we should be able to replace all the reference stream and smart by fuel.
Yes, but Fuel is not completly ready to fully replace them. There are some hooks they provide that Fuel does not yet. But we will get there at some point.
stef
-- Mariano http://marianopeck.wordpress.com
On Dec 11, 2011, at 4:54 PM, Mariano Martinez Peck wrote:
On Sun, Dec 11, 2011 at 4:35 PM, Stéphane Ducasse <stephane.ducasse@inria.fr> wrote:
On Dec 11, 2011, at 4:26 PM, Mariano Martinez Peck wrote:
On Sun, Dec 11, 2011 at 11:32 AM, Stéphane Ducasse <stephane.ducasse@inria.fr> wrote: Indeed this is fun I was doing the same :) Now I do not understand why some parts of the system uses dataStream, others ReferenceStream and finally others SmartRefStream why not all of them use SmartRefStream.
It is not that I like these guys, but their class comments are pretty clear: - DataStream does NOT handle cycles, but hence it may be faster in certain scenarios. I guess that for example, Monticello is sure that in its definitions, there cannot be cycles. In addition, it doesn't take into account variables names/order and hence class reshape isn't supported. - ReferenceStream supports cycles but still doesn't support class reshape. - SmartRefStream supports cycles and class reshape.
sounds good
Yes it does. In fact, I would love also to have those things optional/configurable in Fuel: optional cycle detection and optional class reshape support :)
then we should be able to replace all the reference stream and smart by fuel.
Yes, but Fuel is not completly ready to fully replace them. There are some hooks they provide that Fuel does not yet. But we will get there at some point.
we will have because we do not want to stay indefinitely with image segment and smart ref. :)
stef
-- Mariano http://marianopeck.wordpress.com
When I last looked at SmartReferenceStream, it did two things that were bad: (1) it essentially assumed all "users" are in fact programmers, have access to the debugger, and (more presumptuous) know what to to with same. There are times when yelling "it's broken" is about all the system should dare do. SRS made no such distinction. Again, the age of the code is showing. We have exception handling now, and should design with it in mind. (2) reshape code was placed in SRS itself, not in the class that is changing shape. The thing that is being deserialized (which is when problems become apparent) should be in charge of the change. It simplifies both packaging (fewer loose methods to consider, and reduced opportunity for name collisions) and it also helps with using proxies for special circumstances. ________________________________ From: pharo-project-bounces@lists.gforge.inria.fr [pharo-project-bounces@lists.gforge.inria.fr] on behalf of Mariano Martinez Peck [marianopeck@gmail.com] Sent: Sunday, December 11, 2011 10:54 AM To: Pharo-project@lists.gforge.inria.fr Subject: Re: [Pharo-project] Cleaning up SmartRefStream use On Sun, Dec 11, 2011 at 4:35 PM, Stéphane Ducasse <stephane.ducasse@inria.fr<mailto:stephane.ducasse@inria.fr>> wrote: On Dec 11, 2011, at 4:26 PM, Mariano Martinez Peck wrote:
On Sun, Dec 11, 2011 at 11:32 AM, Stéphane Ducasse <stephane.ducasse@inria.fr<mailto:stephane.ducasse@inria.fr>> wrote: Indeed this is fun I was doing the same :) Now I do not understand why some parts of the system uses dataStream, others ReferenceStream and finally others SmartRefStream why not all of them use SmartRefStream.
It is not that I like these guys, but their class comments are pretty clear: - DataStream does NOT handle cycles, but hence it may be faster in certain scenarios. I guess that for example, Monticello is sure that in its definitions, there cannot be cycles. In addition, it doesn't take into account variables names/order and hence class reshape isn't supported. - ReferenceStream supports cycles but still doesn't support class reshape. - SmartRefStream supports cycles and class reshape.
sounds good Yes it does. In fact, I would love also to have those things optional/configurable in Fuel: optional cycle detection and optional class reshape support :) then we should be able to replace all the reference stream and smart by fuel. Yes, but Fuel is not completly ready to fully replace them. There are some hooks they provide that Fuel does not yet. But we will get there at some point. stef -- Mariano http://marianopeck.wordpress.com
On Sun, Dec 11, 2011 at 7:50 PM, Schwab,Wilhelm K <bschwab@anest.ufl.edu>wrote:
When I last looked at SmartReferenceStream, it did two things that were bad:
(1) it essentially assumed all "users" are in fact programmers, have access to the debugger, and (more presumptuous) know what to to with same. There are times when yelling "it's broken" is about all the system should dare do. SRS made no such distinction. Again, the age of the code is showing. We have exception handling now, and should design with it in mind.
(2) reshape code was placed in SRS itself, not in the class that is changing shape. The thing that is being deserialized (which is when problems become apparent) should be in charge of the change. It simplifies both packaging (fewer loose methods to consider, and reduced opportunity for name collisions) and it also helps with using proxies for special circumstances.
Thanks for the notes Bill. We will take them into account.
------------------------------ *From:* pharo-project-bounces@lists.gforge.inria.fr [ pharo-project-bounces@lists.gforge.inria.fr] on behalf of Mariano Martinez Peck [marianopeck@gmail.com] *Sent:* Sunday, December 11, 2011 10:54 AM *To:* Pharo-project@lists.gforge.inria.fr *Subject:* Re: [Pharo-project] Cleaning up SmartRefStream use
On Sun, Dec 11, 2011 at 4:35 PM, Stéphane Ducasse < stephane.ducasse@inria.fr> wrote:
On Dec 11, 2011, at 4:26 PM, Mariano Martinez Peck wrote:
On Sun, Dec 11, 2011 at 11:32 AM, Stéphane Ducasse <
stephane.ducasse@inria.fr> wrote:
Indeed this is fun I was doing the same :) Now I do not understand why some parts of the system uses dataStream, others ReferenceStream and finally others SmartRefStream why not all of them use SmartRefStream.
It is not that I like these guys, but their class comments are pretty clear: - DataStream does NOT handle cycles, but hence it may be faster in certain scenarios. I guess that for example, Monticello is sure that in its definitions, there cannot be cycles. In addition, it doesn't take into account variables names/order and hence class reshape isn't supported. - ReferenceStream supports cycles but still doesn't support class reshape. - SmartRefStream supports cycles and class reshape.
sounds good
Yes it does. In fact, I would love also to have those things optional/configurable in Fuel: optional cycle detection and optional class reshape support :)
then we should be able to replace all the reference stream and smart by fuel.
Yes, but Fuel is not completly ready to fully replace them. There are some hooks they provide that Fuel does not yet. But we will get there at some point.
stef
-- Mariano http://marianopeck.wordpress.com
-- Mariano http://marianopeck.wordpress.com
On 11.12.2011 19:50, Schwab,Wilhelm K wrote:
When I last looked at SmartReferenceStream, it did two things that were bad:
(2) reshape code was placed in SRS itself, not in the class that is changing shape. The thing that is being deserialized (which is when problems become apparent) should be in charge of the change. It simplifies both packaging (fewer loose methods to consider, and reduced opportunity for name collisions) and it also helps with using proxies for special circumstances.
I'm not sure I understand/agree with what you are saying. What has been serialized is (almost) always an older version than what you are loading into, in which case I can't really see how it can answer the question "How do I turn into the current version?" correctly. Maybe there needs to be a split? Serialized version knows how to install itself as older versions (incase that's what you load into), while the image takes care if version in memory is newer than serialized? TBH, I also feel it's better to put the "import old versions" functionality as methods on the class itself rather than have an omnipotent materializer with special methods for installing all versions of all known classes in the system, I mean, exactly in the serializer/materializer do you intend to put such logic for custom user classes? Cheers, Henry
Henry, Of course the old data do no know how to turn themselves into the current format. But they do/should carry with them version information (can be as simple as an integer bumped with each change); the system uses that to tell which incremental reshapes to apply. It works like a charm. Putting the conversion methods in the deserializer is _possible_, but it becomes a huge mess over time, risks collisions, complicates packaging for everything that uses it, and the code is FAR better placed in the classes that are changing over time. The latter becomes really obvious when one starts to use proxies. Bill ________________________________ From: pharo-project-bounces@lists.gforge.inria.fr [pharo-project-bounces@lists.gforge.inria.fr] on behalf of Henrik Sperre Johansen [henrik.s.johansen@veloxit.no] Sent: Tuesday, December 13, 2011 5:55 AM To: Pharo-project@lists.gforge.inria.fr Subject: Re: [Pharo-project] Cleaning up SmartRefStream use On 11.12.2011 19:50, Schwab,Wilhelm K wrote: When I last looked at SmartReferenceStream, it did two things that were bad: (2) reshape code was placed in SRS itself, not in the class that is changing shape. The thing that is being deserialized (which is when problems become apparent) should be in charge of the change. It simplifies both packaging (fewer loose methods to consider, and reduced opportunity for name collisions) and it also helps with using proxies for special circumstances. I'm not sure I understand/agree with what you are saying. What has been serialized is (almost) always an older version than what you are loading into, in which case I can't really see how it can answer the question "How do I turn into the current version?" correctly. Maybe there needs to be a split? Serialized version knows how to install itself as older versions (incase that's what you load into), while the image takes care if version in memory is newer than serialized? TBH, I also feel it's better to put the "import old versions" functionality as methods on the class itself rather than have an omnipotent materializer with special methods for installing all versions of all known classes in the system, I mean, exactly in the serializer/materializer do you intend to put such logic for custom user classes? Cheers, Henry
On 13.12.2011 15:35, Schwab,Wilhelm K wrote:
Henry,
Of course the old data do no know how to turn themselves into the current format. But they do/should carry with them version information (can be as simple as an integer bumped with each change); the system uses that to tell which incremental reshapes to apply. It works like a charm.
Putting the conversion methods in the deserializer is _possible_, but it becomes a huge mess over time, risks collisions, complicates packaging for everything that uses it, and the code is FAR better placed in the classes that are changing over time. The latter becomes really obvious when one starts to use proxies.
Bill
Then we agree on all points, I just misread what you'd written :) Cheers, Henry
------------------------------------------------------------------------ *From:* pharo-project-bounces@lists.gforge.inria.fr [pharo-project-bounces@lists.gforge.inria.fr] on behalf of Henrik Sperre Johansen [henrik.s.johansen@veloxit.no] *Sent:* Tuesday, December 13, 2011 5:55 AM *To:* Pharo-project@lists.gforge.inria.fr *Subject:* Re: [Pharo-project] Cleaning up SmartRefStream use
On 11.12.2011 19:50, Schwab,Wilhelm K wrote:
When I last looked at SmartReferenceStream, it did two things that were bad:
(2) reshape code was placed in SRS itself, not in the class that is changing shape. The thing that is being deserialized (which is when problems become apparent) should be in charge of the change. It simplifies both packaging (fewer loose methods to consider, and reduced opportunity for name collisions) and it also helps with using proxies for special circumstances.
I'm not sure I understand/agree with what you are saying. What has been serialized is (almost) always an older version than what you are loading into, in which case I can't really see how it can answer the question "How do I turn into the current version?" correctly. Maybe there needs to be a split? Serialized version knows how to install itself as older versions (incase that's what you load into), while the image takes care if version in memory is newer than serialized?
TBH, I also feel it's better to put the "import old versions" functionality as methods on the class itself rather than have an omnipotent materializer with special methods for installing all versions of all known classes in the system, I mean, exactly in the serializer/materializer do you intend to put such logic for custom user classes?
Cheers, Henry
Henry, Sounds good :) Bill ________________________________ From: pharo-project-bounces@lists.gforge.inria.fr [pharo-project-bounces@lists.gforge.inria.fr] on behalf of Henrik Sperre Johansen [henrik.s.johansen@veloxit.no] Sent: Tuesday, December 13, 2011 4:12 PM To: Pharo-project@lists.gforge.inria.fr Subject: Re: [Pharo-project] Cleaning up SmartRefStream use On 13.12.2011 15:35, Schwab,Wilhelm K wrote: Henry, Of course the old data do no know how to turn themselves into the current format. But they do/should carry with them version information (can be as simple as an integer bumped with each change); the system uses that to tell which incremental reshapes to apply. It works like a charm. Putting the conversion methods in the deserializer is _possible_, but it becomes a huge mess over time, risks collisions, complicates packaging for everything that uses it, and the code is FAR better placed in the classes that are changing over time. The latter becomes really obvious when one starts to use proxies. Bill Then we agree on all points, I just misread what you'd written :) Cheers, Henry ________________________________ From: pharo-project-bounces@lists.gforge.inria.fr<mailto:pharo-project-bounces@lists.gforge.inria.fr> [pharo-project-bounces@lists.gforge.inria.fr<mailto:pharo-project-bounces@lists.gforge.inria.fr>] on behalf of Henrik Sperre Johansen [henrik.s.johansen@veloxit.no<mailto:henrik.s.johansen@veloxit.no>] Sent: Tuesday, December 13, 2011 5:55 AM To: Pharo-project@lists.gforge.inria.fr<mailto:Pharo-project@lists.gforge.inria.fr> Subject: Re: [Pharo-project] Cleaning up SmartRefStream use On 11.12.2011 19:50, Schwab,Wilhelm K wrote: When I last looked at SmartReferenceStream, it did two things that were bad: (2) reshape code was placed in SRS itself, not in the class that is changing shape. The thing that is being deserialized (which is when problems become apparent) should be in charge of the change. It simplifies both packaging (fewer loose methods to consider, and reduced opportunity for name collisions) and it also helps with using proxies for special circumstances. I'm not sure I understand/agree with what you are saying. What has been serialized is (almost) always an older version than what you are loading into, in which case I can't really see how it can answer the question "How do I turn into the current version?" correctly. Maybe there needs to be a split? Serialized version knows how to install itself as older versions (incase that's what you load into), while the image takes care if version in memory is newer than serialized? TBH, I also feel it's better to put the "import old versions" functionality as methods on the class itself rather than have an omnipotent materializer with special methods for installing all versions of all known classes in the system, I mean, exactly in the serializer/materializer do you intend to put such logic for custom user classes? Cheers, Henry
On Sun, Dec 11, 2011 at 12:54 PM, Mariano Martinez Peck < marianopeck@gmail.com> wrote:
On Sun, Dec 11, 2011 at 4:35 PM, Stéphane Ducasse < stephane.ducasse@inria.fr> wrote:
On Dec 11, 2011, at 4:26 PM, Mariano Martinez Peck wrote:
On Sun, Dec 11, 2011 at 11:32 AM, Stéphane Ducasse <
stephane.ducasse@inria.fr> wrote:
Indeed this is fun I was doing the same :) Now I do not understand why some parts of the system uses dataStream, others ReferenceStream and finally others SmartRefStream why not all of them use SmartRefStream.
It is not that I like these guys, but their class comments are pretty clear: - DataStream does NOT handle cycles, but hence it may be faster in certain scenarios. I guess that for example, Monticello is sure that in its definitions, there cannot be cycles. In addition, it doesn't take into account variables names/order and hence class reshape isn't supported. - ReferenceStream supports cycles but still doesn't support class reshape. - SmartRefStream supports cycles and class reshape.
sounds good
Yes it does. In fact, I would love also to have those things optional/configurable in Fuel: optional cycle detection and optional class reshape support :)
But I don't see any performance cost on "class reshape support", do you?
then we should be able to replace all the reference stream and smart by fuel.
Yes, but Fuel is not completly ready to fully replace them. There are some hooks they provide that Fuel does not yet. But we will get there at some point.
stef
-- Mariano http://marianopeck.wordpress.com
participants (7)
-
Henrik Sperre Johansen -
Marcus Denker -
Mariano Martinez Peck -
Martin Dias -
Nicolas Cellier -
Schwab,Wilhelm K -
Stéphane Ducasse