Re: [Pharo-project] getting rid of Symbol >> new: ?
From: Sheridan Mahoney <sheridan@sheridan-mahoney.com> Date: December 3, 2009 11:04:19 PM GMT+01:00 To: pharo-project-bounces@lists.gforge.inria.fr Subject: Re: getting rid of Symbol >> new: ? Reply-To: sheridan@sheridan-mahoney.com
A colleague and I are investigating the ImageSegment class and its methods, we came across an issue I would like to get external opinions on. Newbie alert, BTW (at least one of us, no names mentioned...). Also, this is not a problem that will affect many users, but it is familiarizing us with the check-in process, slices, etc. While working on ImageSegment tests, we discovered a problem on trying to load segments that had Symbols in the root array. It is possible to create 2 ByteSymbols with the same sequence of characters. :( In trying to track down how this is possible, we came across a side issue, that ByteSymbol new: had the capacity to create multiple new ByteSymbols with the same number of nil characters (as in, initialized with nil). We want to dissallow Symbol new: , which would cause people to use one of the nicer methods for Symbol/ByteSymbol creation (namely, one which checks that the sequence of characters doesn't already exist, as part of the creation process). We have a fix we want to check in, but currently it breaks a test case in the SymbolTest class, which is checking that new: works. We also changed the intern: method on the class side of Symbol to use basicNew: instead of new: . Are there reasons to keep 'Symbol new:' , that outweigh the reasons to get rid of it? Many thanks, and Cheers, Sheri Mahoney
From: Sheridan Mahoney <sheridan@sheridan-mahoney.com> Date: December 3, 2009 11:04:19 PM GMT+01:00 To: pharo-project-bounces@lists.gforge.inria.fr Subject: Re: getting rid of Symbol >> new: ? Reply-To: sheridan@sheridan-mahoney.com
A colleague and I are investigating the ImageSegment class and its methods, we came across an issue I would like to get external opinions on. Newbie alert, BTW (at least one of us, no names mentioned...). Also, this is not a problem that will affect many users, but it is familiarizing us with the check-in process, slices, etc. While working on ImageSegment tests, we discovered a problem on trying to load segments that had Symbols in the root array. It is possible to create 2 ByteSymbols with the same sequence of characters. :( In trying to track down how this is possible, we came across a side issue, that ByteSymbol new: had the capacity to create multiple new ByteSymbols with the same number of nil characters (as in, initialized with nil). We want to dissallow Symbol new: , which would cause people to use one of the nicer methods for Symbol/ByteSymbol creation (namely, one which checks that the sequence of characters doesn't already exist, as part of the creation process). We have a fix we want to check in, but currently it breaks a test case in the SymbolTest class, which is checking that new: works. We also changed the intern: method on the class side of Symbol to use basicNew: instead of new: . Are there reasons to keep 'Symbol new:' , that outweigh the reasons to get rid of it? Many thanks, and Cheers, Sheri Mahoney
I'm not sure that forbidding new: is a good idea. In Smalltalk it is rare to forbid such methods. Do you know from where your two byteSymbols were coming? Stef
Stéphane Ducasse wrote:
I'm not sure that forbidding new: is a good idea. In Smalltalk it is rare to forbid such methods.
There are actually quite a few examples in existing code where #new and #new: are implemented as #shouldNotImplement or #error:. Classes that require a special way of creating their instances, such as CompiledMethod, often disallow new:. Singleton classes disallow new. Symbols seem to fit the pattern pretty well, and although it doesn't cause huge problems it seems like you shouldn't be able to create an invalid system state by sending public messages like #new:. Regards, -Martin
On Dec 3, 2009, at 2:19 PM, Stéphane Ducasse wrote:
I'm not sure that forbidding new: is a good idea. In Smalltalk it is rare to forbid such methods.
"If an instance created by sending the #new method ... would cause an error, a common practice is to override the behavior of #new so that it fails." Skublics, Klimas, & Thomas, Smalltalk with Style, p. 14 (Prentice Hall, 1996). James Foster
thanks. I know these book Now the question is: are they any senders...
I'm not sure that forbidding new: is a good idea. In Smalltalk it is rare to forbid such methods.
"If an instance created by sending the #new method ... would cause an error, a common practice is to override the behavior of #new so that it fails." Skublics, Klimas, & Thomas, Smalltalk with Style, p. 14 (Prentice Hall, 1996).
James Foster _______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
Hi Sheri, Sounds ok to make new: raise an exception if you adjust the tests (and any code that exists in the image using new: (but I assume there is none)). However, the actual reason why you get multiple same symbols after loading a segment likely is unrelated to ByteSymbol class>>new:. I guess it is because when creating the segment you do not hold onto these symbols. Like this they do not get into the outPointers ref stream but in the bytearray. When installing the segment again, with same symbols existing in the image already, then you get duplicates. The "right way" to do this is to strongly hold onto all symbols when creating a segment. See #createSegmentFrom:. You can reproduce this problem by commenting out the first line of #createSegmentFrom: and running #testSymbols. Let us know how it goes... BTW, Mariano is also writing ImageSegment tests, so maybe you want to join forces (or maybe he already is the colleague you mentioned?). Cheers, Adrian On Dec 3, 2009, at 23:10 , Stéphane Ducasse wrote:
From: Sheridan Mahoney <sheridan@sheridan-mahoney.com> Date: December 3, 2009 11:04:19 PM GMT+01:00 To: pharo-project-bounces@lists.gforge.inria.fr Subject: Re: getting rid of Symbol >> new: ? Reply-To: sheridan@sheridan-mahoney.com
A colleague and I are investigating the ImageSegment class and its methods, we came across an issue I would like to get external opinions on. Newbie alert, BTW (at least one of us, no names mentioned...). Also, this is not a problem that will affect many users, but it is familiarizing us with the check-in process, slices, etc. While working on ImageSegment tests, we discovered a problem on trying to load segments that had Symbols in the root array. It is possible to create 2 ByteSymbols with the same sequence of characters. :( In trying to track down how this is possible, we came across a side issue, that ByteSymbol new: had the capacity to create multiple new ByteSymbols with the same number of nil characters (as in, initialized with nil). We want to dissallow Symbol new: , which would cause people to use one of the nicer methods for Symbol/ByteSymbol creation (namely, one which checks that the sequence of characters doesn't already exist, as part of the creation process). We have a fix we want to check in, but currently it breaks a test case in the SymbolTest class, which is checking that new: works. We also changed the intern: method on the class side of Symbol to use basicNew: instead of new: . Are there reasons to keep 'Symbol new:' , that outweigh the reasons to get rid of it? Many thanks, and Cheers, Sheri Mahoney
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
Great! The other person helping me is Martin McClure, and Mariano contacted me just yesterday to start collaborating, so looks like there are 3 of us now hooked on ImageSegment.... -- Sheri Adrian Lienhard wrote:
Hi Sheri,
Sounds ok to make new: raise an exception if you adjust the tests (and any code that exists in the image using new: (but I assume there is none)).
However, the actual reason why you get multiple same symbols after loading a segment likely is unrelated to ByteSymbol class>>new:. I guess it is because when creating the segment you do not hold onto these symbols. Like this they do not get into the outPointers ref stream but in the bytearray. When installing the segment again, with same symbols existing in the image already, then you get duplicates.
The "right way" to do this is to strongly hold onto all symbols when creating a segment. See #createSegmentFrom:. You can reproduce this problem by commenting out the first line of #createSegmentFrom: and running #testSymbols.
Let us know how it goes...
BTW, Mariano is also writing ImageSegment tests, so maybe you want to join forces (or maybe he already is the colleague you mentioned?).
Cheers, Adrian
On Dec 3, 2009, at 23:10 , Stéphane Ducasse wrote:
From: Sheridan Mahoney <sheridan@sheridan-mahoney.com> Date: December 3, 2009 11:04:19 PM GMT+01:00 To: pharo-project-bounces@lists.gforge.inria.fr Subject: Re: getting rid of Symbol >> new: ? Reply-To: sheridan@sheridan-mahoney.com
A colleague and I are investigating the ImageSegment class and its methods, we came across an issue I would like to get external opinions on. Newbie alert, BTW (at least one of us, no names mentioned...). Also, this is not a problem that will affect many users, but it is familiarizing us with the check-in process, slices, etc. While working on ImageSegment tests, we discovered a problem on trying to load segments that had Symbols in the root array. It is possible to create 2 ByteSymbols with the same sequence of characters. :( In trying to track down how this is possible, we came across a side issue, that ByteSymbol new: had the capacity to create multiple new ByteSymbols with the same number of nil characters (as in, initialized with nil). We want to dissallow Symbol new: , which would cause people to use one of the nicer methods for Symbol/ByteSymbol creation (namely, one which checks that the sequence of characters doesn't already exist, as part of the creation process). We have a fix we want to check in, but currently it breaks a test case in the SymbolTest class, which is checking that new: works. We also changed the intern: method on the class side of Symbol to use basicNew: instead of new: . Are there reasons to keep 'Symbol new:' , that outweigh the reasons to get rid of it? Many thanks, and Cheers, Sheri Mahoney
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
-- View this message in context: http://n2.nabble.com/Re-getting-rid-of-Symbol-new-tp4109230p4116203.html Sent from the Pharo Smalltalk mailing list archive at Nabble.com.
Excellent! We need really cool and well tested imageSegments without etoy and project refs everywhere. Let us know up to date. On Dec 5, 2009, at 3:17 AM, Sheridan Mahoney wrote:
Great! The other person helping me is Martin McClure, and Mariano contacted me just yesterday to start collaborating, so looks like there are 3 of us now hooked on ImageSegment....
-- Sheri
Adrian Lienhard wrote:
Hi Sheri,
Sounds ok to make new: raise an exception if you adjust the tests (and any code that exists in the image using new: (but I assume there is none)).
However, the actual reason why you get multiple same symbols after loading a segment likely is unrelated to ByteSymbol class>>new:. I guess it is because when creating the segment you do not hold onto these symbols. Like this they do not get into the outPointers ref stream but in the bytearray. When installing the segment again, with same symbols existing in the image already, then you get duplicates.
The "right way" to do this is to strongly hold onto all symbols when creating a segment. See #createSegmentFrom:. You can reproduce this problem by commenting out the first line of #createSegmentFrom: and running #testSymbols.
Let us know how it goes...
BTW, Mariano is also writing ImageSegment tests, so maybe you want to join forces (or maybe he already is the colleague you mentioned?).
Cheers, Adrian
On Dec 3, 2009, at 23:10 , Stéphane Ducasse wrote:
From: Sheridan Mahoney <sheridan@sheridan-mahoney.com> Date: December 3, 2009 11:04:19 PM GMT+01:00 To: pharo-project-bounces@lists.gforge.inria.fr Subject: Re: getting rid of Symbol >> new: ? Reply-To: sheridan@sheridan-mahoney.com
A colleague and I are investigating the ImageSegment class and its methods, we came across an issue I would like to get external opinions on. Newbie alert, BTW (at least one of us, no names mentioned...). Also, this is not a problem that will affect many users, but it is familiarizing us with the check-in process, slices, etc. While working on ImageSegment tests, we discovered a problem on trying to load segments that had Symbols in the root array. It is possible to create 2 ByteSymbols with the same sequence of characters. :( In trying to track down how this is possible, we came across a side issue, that ByteSymbol new: had the capacity to create multiple new ByteSymbols with the same number of nil characters (as in, initialized with nil). We want to dissallow Symbol new: , which would cause people to use one of the nicer methods for Symbol/ByteSymbol creation (namely, one which checks that the sequence of characters doesn't already exist, as part of the creation process). We have a fix we want to check in, but currently it breaks a test case in the SymbolTest class, which is checking that new: works. We also changed the intern: method on the class side of Symbol to use basicNew: instead of new: . Are there reasons to keep 'Symbol new:' , that outweigh the reasons to get rid of it? Many thanks, and Cheers, Sheri Mahoney
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
-- View this message in context: http://n2.nabble.com/Re-getting-rid-of-Symbol-new-tp4109230p4116203.html Sent from the Pharo Smalltalk mailing list archive at Nabble.com.
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
I should relate our experience with segments in Sophie was not helpful. We attempted to segment off our font menu since it was expensive to build, and really only needed to be changed if the fonts on the machine changed. Since the image was read only we would startup, build the font menu then image segment that off. On a restart we just read the segment in, confirmed the machine didn't have font files changed. This worked well in the *lab*. But when we push it to the public we started getting email about machines crashing. Tim and I were just unable to determine why... But it was always related to the point where it read the image segment, sometimes it would crash (rarely) mostly not. We backed that out, and settled with a image segment that really just stored forms of each font face for the menu. That *seemed* to work ok On 2009-12-05, at 6:53 AM, Stéphane Ducasse wrote:
Excellent! We need really cool and well tested imageSegments without etoy and project refs everywhere. Let us know up to date.
On Dec 5, 2009, at 3:17 AM, Sheridan Mahoney wrote:
Great! The other person helping me is Martin McClure, and Mariano contacted me just yesterday to start collaborating, so looks like there are 3 of us now hooked on ImageSegment....
-- Sheri
Adrian Lienhard wrote:
Hi Sheri,
Sounds ok to make new: raise an exception if you adjust the tests (and any code that exists in the image using new: (but I assume there is none)).
However, the actual reason why you get multiple same symbols after loading a segment likely is unrelated to ByteSymbol class>>new:. I guess it is because when creating the segment you do not hold onto these symbols. Like this they do not get into the outPointers ref stream but in the bytearray. When installing the segment again, with same symbols existing in the image already, then you get duplicates.
The "right way" to do this is to strongly hold onto all symbols when creating a segment. See #createSegmentFrom:. You can reproduce this problem by commenting out the first line of #createSegmentFrom: and running #testSymbols.
Let us know how it goes...
BTW, Mariano is also writing ImageSegment tests, so maybe you want to join forces (or maybe he already is the colleague you mentioned?).
Cheers, Adrian
On Dec 3, 2009, at 23:10 , Stéphane Ducasse wrote:
From: Sheridan Mahoney <sheridan@sheridan-mahoney.com> Date: December 3, 2009 11:04:19 PM GMT+01:00 To: pharo-project-bounces@lists.gforge.inria.fr Subject: Re: getting rid of Symbol >> new: ? Reply-To: sheridan@sheridan-mahoney.com
A colleague and I are investigating the ImageSegment class and its methods, we came across an issue I would like to get external opinions on. Newbie alert, BTW (at least one of us, no names mentioned...). Also, this is not a problem that will affect many users, but it is familiarizing us with the check-in process, slices, etc. While working on ImageSegment tests, we discovered a problem on trying to load segments that had Symbols in the root array. It is possible to create 2 ByteSymbols with the same sequence of characters. :( In trying to track down how this is possible, we came across a side issue, that ByteSymbol new: had the capacity to create multiple new ByteSymbols with the same number of nil characters (as in, initialized with nil). We want to dissallow Symbol new: , which would cause people to use one of the nicer methods for Symbol/ByteSymbol creation (namely, one which checks that the sequence of characters doesn't already exist, as part of the creation process). We have a fix we want to check in, but currently it breaks a test case in the SymbolTest class, which is checking that new: works. We also changed the intern: method on the class side of Symbol to use basicNew: instead of new: . Are there reasons to keep 'Symbol new:' , that outweigh the reasons to get rid of it? Many thanks, and Cheers, Sheri Mahoney
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
-- View this message in context: http://n2.nabble.com/Re-getting-rid-of-Symbol-new-tp4109230p4116203.html Sent from the Pharo Smalltalk mailing list archive at Nabble.com.
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
-- =========================================================================== John M. McIntosh <johnmci@smalltalkconsulting.com> Twitter: squeaker68882 Corporate Smalltalk Consulting Ltd. http://www.smalltalkconsulting.com ===========================================================================
when I see the complexity of the code full of project and morph conditional I imagine that the primary idea beind imageSegment got damaged. Now imageSegment is probably difficult to control too. Stef On Dec 5, 2009, at 9:41 PM, John M McIntosh wrote:
I should relate our experience with segments in Sophie was not helpful.
We attempted to segment off our font menu since it was expensive to build, and really only needed to be changed if the fonts on the machine changed. Since the image was read only we would startup, build the font menu then image segment that off.
On a restart we just read the segment in, confirmed the machine didn't have font files changed. This worked well in the *lab*.
But when we push it to the public we started getting email about machines crashing. Tim and I were just unable to determine why... But it was always related to the point where it read the image segment, sometimes it would crash (rarely) mostly not.
We backed that out, and settled with a image segment that really just stored forms of each font face for the menu. That *seemed* to work ok
On 2009-12-05, at 6:53 AM, Stéphane Ducasse wrote:
Excellent! We need really cool and well tested imageSegments without etoy and project refs everywhere. Let us know up to date.
On Dec 5, 2009, at 3:17 AM, Sheridan Mahoney wrote:
Great! The other person helping me is Martin McClure, and Mariano contacted me just yesterday to start collaborating, so looks like there are 3 of us now hooked on ImageSegment....
-- Sheri
Adrian Lienhard wrote:
Hi Sheri,
Sounds ok to make new: raise an exception if you adjust the tests (and any code that exists in the image using new: (but I assume there is none)).
However, the actual reason why you get multiple same symbols after loading a segment likely is unrelated to ByteSymbol class>>new:. I guess it is because when creating the segment you do not hold onto these symbols. Like this they do not get into the outPointers ref stream but in the bytearray. When installing the segment again, with same symbols existing in the image already, then you get duplicates.
The "right way" to do this is to strongly hold onto all symbols when creating a segment. See #createSegmentFrom:. You can reproduce this problem by commenting out the first line of #createSegmentFrom: and running #testSymbols.
Let us know how it goes...
BTW, Mariano is also writing ImageSegment tests, so maybe you want to join forces (or maybe he already is the colleague you mentioned?).
Cheers, Adrian
On Dec 3, 2009, at 23:10 , Stéphane Ducasse wrote:
From: Sheridan Mahoney <sheridan@sheridan-mahoney.com> Date: December 3, 2009 11:04:19 PM GMT+01:00 To: pharo-project-bounces@lists.gforge.inria.fr Subject: Re: getting rid of Symbol >> new: ? Reply-To: sheridan@sheridan-mahoney.com
A colleague and I are investigating the ImageSegment class and its methods, we came across an issue I would like to get external opinions on. Newbie alert, BTW (at least one of us, no names mentioned...). Also, this is not a problem that will affect many users, but it is familiarizing us with the check-in process, slices, etc. While working on ImageSegment tests, we discovered a problem on trying to load segments that had Symbols in the root array. It is possible to create 2 ByteSymbols with the same sequence of characters. :( In trying to track down how this is possible, we came across a side issue, that ByteSymbol new: had the capacity to create multiple new ByteSymbols with the same number of nil characters (as in, initialized with nil). We want to dissallow Symbol new: , which would cause people to use one of the nicer methods for Symbol/ByteSymbol creation (namely, one which checks that the sequence of characters doesn't already exist, as part of the creation process). We have a fix we want to check in, but currently it breaks a test case in the SymbolTest class, which is checking that new: works. We also changed the intern: method on the class side of Symbol to use basicNew: instead of new: . Are there reasons to keep 'Symbol new:' , that outweigh the reasons to get rid of it? Many thanks, and Cheers, Sheri Mahoney
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
-- View this message in context: http://n2.nabble.com/Re-getting-rid-of-Symbol-new-tp4109230p4116203.html Sent from the Pharo Smalltalk mailing list archive at Nabble.com.
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
-- =========================================================================== John M. McIntosh <johnmci@smalltalkconsulting.com> Twitter: squeaker68882 Corporate Smalltalk Consulting Ltd. http://www.smalltalkconsulting.com ===========================================================================
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
On Sat, Dec 5, 2009 at 9:41 PM, John M McIntosh < johnmci@smalltalkconsulting.com> wrote:
I should relate our experience with segments in Sophie was not helpful.
We attempted to segment off our font menu since it was expensive to build, and really only needed to be changed if the fonts on the machine changed. Since the image was read only we would startup, build the font menu then image segment that off.
On a restart we just read the segment in, confirmed the machine didn't have font files changed. This worked well in the *lab*.
But when we push it to the public we started getting email about machines crashing. Tim and I were just unable to determine why... But it was always related to the point where it read the image segment, sometimes it would crash (rarely) mostly not.
We backed that out, and settled with a image segment that really just stored forms of each font face for the menu. That *seemed* to work ok
Thanks John. I am interested in your experience. However, I didn't understand this last paragraph where you said to finally make it work. I don't understand what did you change. What is the difference between "font menu" and " stored forms of each font face for the menu" ? In addition, do you know why this has solved the problems ? Best regards, Mariano
On 2009-12-05, at 6:53 AM, Stéphane Ducasse wrote:
Excellent! We need really cool and well tested imageSegments without etoy and project refs everywhere. Let us know up to date.
On Dec 5, 2009, at 3:17 AM, Sheridan Mahoney wrote:
Great! The other person helping me is Martin McClure, and Mariano
contacted
me just yesterday to start collaborating, so looks like there are 3 of us now hooked on ImageSegment....
-- Sheri
Adrian Lienhard wrote:
Hi Sheri,
Sounds ok to make new: raise an exception if you adjust the tests (and any code that exists in the image using new: (but I assume there is none)).
However, the actual reason why you get multiple same symbols after loading a segment likely is unrelated to ByteSymbol class>>new:. I guess it is because when creating the segment you do not hold onto these symbols. Like this they do not get into the outPointers ref stream but in the bytearray. When installing the segment again, with same symbols existing in the image already, then you get duplicates.
The "right way" to do this is to strongly hold onto all symbols when creating a segment. See #createSegmentFrom:. You can reproduce this problem by commenting out the first line of #createSegmentFrom: and running #testSymbols.
Let us know how it goes...
BTW, Mariano is also writing ImageSegment tests, so maybe you want to join forces (or maybe he already is the colleague you mentioned?).
Cheers, Adrian
On Dec 3, 2009, at 23:10 , Stéphane Ducasse wrote:
From: Sheridan Mahoney <sheridan@sheridan-mahoney.com> Date: December 3, 2009 11:04:19 PM GMT+01:00 To: pharo-project-bounces@lists.gforge.inria.fr Subject: Re: getting rid of Symbol >> new: ? Reply-To: sheridan@sheridan-mahoney.com
A colleague and I are investigating the ImageSegment class and its methods, we came across an issue I would like to get external opinions on. Newbie alert, BTW (at least one of us, no names mentioned...). Also, this is not a problem that will affect many users, but it is familiarizing us with the check-in process, slices, etc. While working on ImageSegment tests, we discovered a problem on trying to load segments that had Symbols in the root array. It is possible to create 2 ByteSymbols with the same sequence of characters. :( In trying to track down how this is possible, we came across a side issue, that ByteSymbol new: had the capacity to create multiple new ByteSymbols with the same number of nil characters (as in, initialized with nil). We want to dissallow Symbol new: , which would cause people to use one of the nicer methods for Symbol/ByteSymbol creation (namely, one which checks that the sequence of characters doesn't already exist, as part of the creation process). We have a fix we want to check in, but currently it breaks a test case in the SymbolTest class, which is checking that new: works. We also changed the intern: method on the class side of Symbol to use basicNew: instead of new: . Are there reasons to keep 'Symbol new:' , that outweigh the reasons to get rid of it? Many thanks, and Cheers, Sheri Mahoney
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
-- View this message in context: http://n2.nabble.com/Re-getting-rid-of-Symbol-new-tp4109230p4116203.html Sent from the Pharo Smalltalk mailing list archive at Nabble.com.
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
-- =========================================================================== John M. McIntosh <johnmci@smalltalkconsulting.com> Twitter: squeaker68882 Corporate Smalltalk Consulting Ltd. http://www.smalltalkconsulting.com ===========================================================================
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
On Dec 8, 2009, at 11:33 20AM, Mariano Martinez Peck wrote:
Thanks John. I am interested in your experience. However, I didn't understand this last paragraph where you said to finally make it work. I don't understand what did you change. What is the difference between "font menu" and " stored forms of each font face for the menu" ?
The way I read it, instead of storing the fonts themselves in an image segment, they stored a form with a string rendered by the font, then used that to display in list instead of actually loading the font, and rendering text with it. That way the list could be shown fast, without having to load the fonts themselves until actually used/fonts on the machine changes.
In addition, do you know why this has solved the problems ?
If you look at the relative complexity of TTCFont and Form instances, it'd hardly come as a surprise that one may be loaded consistently, while the other is harder to get loaded correctly in all cases... In other words, to me it seems like a workaround for non-repeatable errors when loading complex objects within ImageSegments (by storing simpler objects instead). Cheers, Henry
The chore of loading all 500 fonts on a macintosh, rendering them all to show what they look like, was *quite* expensive and added many 10's of seconds to Sophie startup time. At first we dumped the entire font menu oops tree into a image segment. The did a merge process by checking the current machine's view of the fonts, versus our historical view. usually fonts don't change much so the cost was only reading the directory. This worked oh, 99.99% of the time. The 0.01% was the hassle. Our fallback was then to store the rendered form for each font in an image segment. That seemed to work 100% As pointed out there is just a mess of eToys/Projects/whatever special casing going on in the default storage/reading of the segments, and when it goes bad, it's *hard* to figure out why. I did explore this area a bit more with WikiServer. In fact it's still on my to-do list, you can customize/override things to get a more simplistic set of steps, but I wasn't quite sure that sending a Pier Wiki to an image segment would be readable in the future, so I decided not to offer that as a storage option. On 2009-12-08, at 3:04 AM, Henrik Johansen wrote:
On Dec 8, 2009, at 11:33 20AM, Mariano Martinez Peck wrote:
Thanks John. I am interested in your experience. However, I didn't understand this last paragraph where you said to finally make it work. I don't understand what did you change. What is the difference between "font menu" and " stored forms of each font face for the menu" ?
The way I read it, instead of storing the fonts themselves in an image segment, they stored a form with a string rendered by the font, then used that to display in list instead of actually loading the font, and rendering text with it. That way the list could be shown fast, without having to load the fonts themselves until actually used/fonts on the machine changes.
In addition, do you know why this has solved the problems ?
If you look at the relative complexity of TTCFont and Form instances, it'd hardly come as a surprise that one may be loaded consistently, while the other is harder to get loaded correctly in all cases... In other words, to me it seems like a workaround for non-repeatable errors when loading complex objects within ImageSegments (by storing simpler objects instead).
Cheers, Henry _______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
-- =========================================================================== John M. McIntosh <johnmci@smalltalkconsulting.com> Twitter: squeaker68882 Corporate Smalltalk Consulting Ltd. http://www.smalltalkconsulting.com ===========================================================================
This is just to let the people of the list know I have reported the above bug (Issue 1551) and submitted a fix and tests to PharoInbox. It is called: SLICE-disallow-Symbol-new-SheridanMahoney.2 I didn't see a way to change the status to "Fixed", so it is just listed as "New". Cheers, Sheri Adrian Lienhard wrote:
Hi Sheri,
Sounds ok to make new: raise an exception if you adjust the tests (and any code that exists in the image using new: (but I assume there is none)).
-- View this message in context: http://n2.nabble.com/Re-getting-rid-of-Symbol-new-tp4109230p4127821.html Sent from the Pharo Smalltalk mailing list archive at Nabble.com.
and, forgot to mention, I did an image check, and did not find any uses of Symbol >> new: once we changed Symbol >> intern: . Also, does something need to be enabled in order to be able to modify the description fields in the issue tracker? Not having much luck in my searches... Thanks, Sheri Sheridan Mahoney wrote:
This is just to let the people of the list know I have reported the above bug (Issue 1551) and submitted a fix and tests to PharoInbox. It is called: SLICE-disallow-Symbol-new-SheridanMahoney.2
I didn't see a way to change the status to "Fixed", so it is just listed as "New".
Cheers, Sheri
Adrian Lienhard wrote:
Hi Sheri,
Sounds ok to make new: raise an exception if you adjust the tests (and any code that exists in the image using new: (but I assume there is none)).
-- View this message in context: http://n2.nabble.com/Re-getting-rid-of-Symbol-new-tp4109230p4128207.html Sent from the Pharo Smalltalk mailing list archive at Nabble.com.
Hi Sheri, Thanks for the contribution! Please send me the mail address you use for the Google account. I'll add you to the list of developers. BTW, have you signed the license agreement? Cheers, Adrian On Dec 7, 2009, at 21:14 , Sheridan Mahoney wrote:
and, forgot to mention, I did an image check, and did not find any uses of Symbol >> new: once we changed Symbol >> intern: .
Also, does something need to be enabled in order to be able to modify the description fields in the issue tracker? Not having much luck in my searches...
Thanks, Sheri
Sheridan Mahoney wrote:
This is just to let the people of the list know I have reported the above bug (Issue 1551) and submitted a fix and tests to PharoInbox. It is called: SLICE-disallow-Symbol-new-SheridanMahoney.2
I didn't see a way to change the status to "Fixed", so it is just listed as "New".
Cheers, Sheri
Adrian Lienhard wrote:
Hi Sheri,
Sounds ok to make new: raise an exception if you adjust the tests (and any code that exists in the image using new: (but I assume there is none)).
-- View this message in context: http://n2.nabble.com/Re-getting-rid-of-Symbol-new-tp4109230p4128207.html Sent from the Pharo Smalltalk mailing list archive at Nabble.com.
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
The address for my Google account is: smagooey@yahoo.com I just sent Stef an email attachment with the license agreement, and will be putting a hard copy in the mail. Cheers, Sheri Adrian Lienhard wrote:
Hi Sheri,
Thanks for the contribution!
Please send me the mail address you use for the Google account. I'll add you to the list of developers.
BTW, have you signed the license agreement?
Cheers, Adrian
On Dec 7, 2009, at 21:14 , Sheridan Mahoney wrote:
and, forgot to mention, I did an image check, and did not find any uses of Symbol >> new: once we changed Symbol >> intern: .
Also, does something need to be enabled in order to be able to modify the description fields in the issue tracker? Not having much luck in my searches...
Thanks, Sheri
Sheridan Mahoney wrote:
This is just to let the people of the list know I have reported the above bug (Issue 1551) and submitted a fix and tests to PharoInbox. It is called: SLICE-disallow-Symbol-new-SheridanMahoney.2
I didn't see a way to change the status to "Fixed", so it is just listed as "New".
Cheers, Sheri
Adrian Lienhard wrote:
Hi Sheri,
Sounds ok to make new: raise an exception if you adjust the tests (and any code that exists in the image using new: (but I assume there is none)).
-- View this message in context: http://n2.nabble.com/Re-getting-rid-of-Symbol-new-tp4109230p4128207.html Sent from the Pharo Smalltalk mailing list archive at Nabble.com.
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
-- View this message in context: http://n2.nabble.com/Re-getting-rid-of-Symbol-new-tp4109230p4128749.html Sent from the Pharo Smalltalk mailing list archive at Nabble.com.
I added you as a project committer. Cheers, Adrian On Dec 7, 2009, at 22:55 , Sheridan Mahoney wrote:
The address for my Google account is:
smagooey@yahoo.com
I just sent Stef an email attachment with the license agreement, and will be putting a hard copy in the mail.
Cheers, Sheri
Adrian Lienhard wrote:
Hi Sheri,
Thanks for the contribution!
Please send me the mail address you use for the Google account. I'll add you to the list of developers.
BTW, have you signed the license agreement?
Cheers, Adrian
On Dec 7, 2009, at 21:14 , Sheridan Mahoney wrote:
and, forgot to mention, I did an image check, and did not find any uses of Symbol >> new: once we changed Symbol >> intern: .
Also, does something need to be enabled in order to be able to modify the description fields in the issue tracker? Not having much luck in my searches...
Thanks, Sheri
Sheridan Mahoney wrote:
This is just to let the people of the list know I have reported the above bug (Issue 1551) and submitted a fix and tests to PharoInbox. It is called: SLICE-disallow-Symbol-new-SheridanMahoney.2
I didn't see a way to change the status to "Fixed", so it is just listed as "New".
Cheers, Sheri
Adrian Lienhard wrote:
Hi Sheri,
Sounds ok to make new: raise an exception if you adjust the tests (and any code that exists in the image using new: (but I assume there is none)).
-- View this message in context: http://n2.nabble.com/Re-getting-rid-of-Symbol-new-tp4109230p4128207.html Sent from the Pharo Smalltalk mailing list archive at Nabble.com.
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
-- View this message in context: http://n2.nabble.com/Re-getting-rid-of-Symbol-new-tp4109230p4128749.html Sent from the Pharo Smalltalk mailing list archive at Nabble.com.
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
On Fri, Dec 4, 2009 at 12:05 PM, Adrian Lienhard <adi@netstyle.ch> wrote:
Hi Sheri,
Sounds ok to make new: raise an exception if you adjust the tests (and any code that exists in the image using new: (but I assume there is none)).
However, the actual reason why you get multiple same symbols after loading a segment likely is unrelated to ByteSymbol class>>new:. I guess it is because when creating the segment you do not hold onto these symbols. Like this they do not get into the outPointers ref stream but in the bytearray. When installing the segment again, with same symbols existing in the image already, then you get duplicates.
The "right way" to do this is to strongly hold onto all symbols when creating a segment. See #createSegmentFrom:. You can reproduce this problem by commenting out the first line of #createSegmentFrom: and running #testSymbols.
Let us know how it goes...
Ok...let me see if I understood you correctly. When you are creating your root of objects and you put symbols inside, they are not put in ourPointers but in ByteArray. This is due to the fact that the only object who is pointing to that symbol is inside the segment ? What you do with this piece of code: symbolHolder := Symbol allSymbols. is to hold those symbols there. So, when ImageSegment uses the GC techniques to detect which objects are ONLY pointed from inside of the segment, the symbols is not found (because it is accessible trough that test) and thus, it goes to outPointers instead of ByteArray. And of course, if it is in outPointers instead of ByteArray when the segment is loaded again, yo don't create a symbol again but use the same object (the one of the oop). I am correct? or I understood anything ? Cheers, Mariano BTW, Mariano is also writing ImageSegment tests, so maybe you want to
join forces (or maybe he already is the colleague you mentioned?).
Cheers, Adrian
On Dec 3, 2009, at 23:10 , Stéphane Ducasse wrote:
From: Sheridan Mahoney <sheridan@sheridan-mahoney.com> Date: December 3, 2009 11:04:19 PM GMT+01:00 To: pharo-project-bounces@lists.gforge.inria.fr Subject: Re: getting rid of Symbol >> new: ? Reply-To: sheridan@sheridan-mahoney.com
A colleague and I are investigating the ImageSegment class and its methods, we came across an issue I would like to get external opinions on. Newbie alert, BTW (at least one of us, no names mentioned...). Also, this is not a problem that will affect many users, but it is familiarizing us with the check-in process, slices, etc. While working on ImageSegment tests, we discovered a problem on trying to load segments that had Symbols in the root array. It is possible to create 2 ByteSymbols with the same sequence of characters. :( In trying to track down how this is possible, we came across a side issue, that ByteSymbol new: had the capacity to create multiple new ByteSymbols with the same number of nil characters (as in, initialized with nil). We want to dissallow Symbol new: , which would cause people to use one of the nicer methods for Symbol/ByteSymbol creation (namely, one which checks that the sequence of characters doesn't already exist, as part of the creation process). We have a fix we want to check in, but currently it breaks a test case in the SymbolTest class, which is checking that new: works. We also changed the intern: method on the class side of Symbol to use basicNew: instead of new: . Are there reasons to keep 'Symbol new:' , that outweigh the reasons to get rid of it? Many thanks, and Cheers, Sheri Mahoney
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
On Dec 8, 2009, at 12:08 , Mariano Martinez Peck wrote:
On Fri, Dec 4, 2009 at 12:05 PM, Adrian Lienhard <adi@netstyle.ch> wrote:
When you are creating your root of objects and you put symbols inside, they are not put in ourPointers but in ByteArray. This is due to the fact that the only object who is pointing to that symbol is inside the segment ?
To be precise, the symbols are also pointed to by the symbol table, but only by weak references. Since image segments use the GC mark logic, these pointers are not considered.
What you do with this piece of code:
symbolHolder := Symbol allSymbols.
is to hold those symbols there. So, when ImageSegment uses the GC techniques to detect which objects are ONLY pointed from inside of the segment, the symbols is not found (because it is accessible trough that test) and thus, it goes to outPointers instead of ByteArray.
And of course, if it is in outPointers instead of ByteArray when the segment is loaded again, yo don't create a symbol again but use the same object (the one of the oop).
I am correct? or I understood anything ?
yes. Cheers, Adrian
On Tue, Dec 8, 2009 at 5:07 PM, Adrian Lienhard <adi@netstyle.ch> wrote:
On Dec 8, 2009, at 12:08 , Mariano Martinez Peck wrote:
On Fri, Dec 4, 2009 at 12:05 PM, Adrian Lienhard <adi@netstyle.ch> wrote:
When you are creating your root of objects and you put symbols inside, they are not put in ourPointers but in ByteArray. This is due to the fact that the only object who is pointing to that symbol is inside the segment ?
To be precise, the symbols are also pointed to by the symbol table, but only by weak references. Since image segments use the GC mark logic, these pointers are not considered.
Ahhh ok. Now I see Symbol class >> rehash where it sets to SymbolTable := WeakSet Now...my finally question is, where in the code you can see the GC only mark "normal" objects and that week objects are not being taken into account. Do you know ? I tried to search it but I didn't find it. Thanks!! Mariano
What you do with this piece of code:
symbolHolder := Symbol allSymbols.
is to hold those symbols there. So, when ImageSegment uses the GC techniques to detect which objects are ONLY pointed from inside of the segment, the symbols is not found (because it is accessible trough that test) and thus, it goes to outPointers instead of ByteArray.
And of course, if it is in outPointers instead of ByteArray when the segment is loaded again, yo don't create a symbol again but use the same object (the one of the oop).
I am correct? or I understood anything ?
yes.
Cheers, Adrian
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
On Tue, Dec 8, 2009 at 10:54 PM, Mariano Martinez Peck < marianopeck@gmail.com> wrote:
On Tue, Dec 8, 2009 at 5:07 PM, Adrian Lienhard <adi@netstyle.ch> wrote:
On Dec 8, 2009, at 12:08 , Mariano Martinez Peck wrote:
On Fri, Dec 4, 2009 at 12:05 PM, Adrian Lienhard <adi@netstyle.ch> wrote:
When you are creating your root of objects and you put symbols inside, they are not put in ourPointers but in ByteArray. This is due to the fact that the only object who is pointing to that symbol is inside the segment ?
To be precise, the symbols are also pointed to by the symbol table, but only by weak references. Since image segments use the GC mark logic, these pointers are not considered.
Ahhh ok. Now I see Symbol class >> rehash where it sets to SymbolTable := WeakSet
Now...my finally question is, where in the code you can see the GC only mark "normal" objects and that week objects are not being taken into account. Do you know ? I tried to search it but I didn't find it.
Sorry, I forgot to said I found this in ObjectMemory >> markAndTrace: (self isWeakNonInt: oop) ifTrue: [ "Set lastFieldOffset before the weak fields in the receiver" lastFieldOffset := (self nonWeakFieldsOf: oop) << ShiftForWord. "And remember as weak root" weakRootCount := weakRootCount + 1. weakRoots at: weakRootCount put: oop. ] ifFalse: [ "Do it the usual way" lastFieldOffset := self lastPointerOf: oop. ]. But I don't know if this make sense or not. Thanks!! Mariano
What you do with this piece of code:
symbolHolder := Symbol allSymbols.
is to hold those symbols there. So, when ImageSegment uses the GC techniques to detect which objects are ONLY pointed from inside of the segment, the symbols is not found (because it is accessible trough that test) and thus, it goes to outPointers instead of ByteArray.
And of course, if it is in outPointers instead of ByteArray when the segment is loaded again, yo don't create a symbol again but use the same object (the one of the oop).
I am correct? or I understood anything ?
yes.
Cheers, Adrian
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
I don't have the VMMaker code at hand right now (or rather, don't have time to look at it ;)), but I guess that after that piece of code you pasted below there is a loop from the first field (BaseHeader+4) to the last non-weak field (lastFieldOffset) in which the non-weak fields are traced. Note, that if an object both fixed and indexable weak fields the latter are always located after the former. You can see the memory layout and object formats on the pages 9 and 10 of the slides [1] (from a lecture about VMs I gave recently). Cheers, Adrian [1] http://www.adrian-lienhard.ch/presentations On Dec 8, 2009, at 22:57 , Mariano Martinez Peck wrote:
On Tue, Dec 8, 2009 at 10:54 PM, Mariano Martinez Peck < marianopeck@gmail.com> wrote:
On Tue, Dec 8, 2009 at 5:07 PM, Adrian Lienhard <adi@netstyle.ch> wrote:
On Dec 8, 2009, at 12:08 , Mariano Martinez Peck wrote:
On Fri, Dec 4, 2009 at 12:05 PM, Adrian Lienhard <adi@netstyle.ch> wrote:
When you are creating your root of objects and you put symbols inside, they are not put in ourPointers but in ByteArray. This is due to the fact that the only object who is pointing to that symbol is inside the segment ?
To be precise, the symbols are also pointed to by the symbol table, but only by weak references. Since image segments use the GC mark logic, these pointers are not considered.
Ahhh ok. Now I see Symbol class >> rehash where it sets to SymbolTable := WeakSet
Now...my finally question is, where in the code you can see the GC only mark "normal" objects and that week objects are not being taken into account. Do you know ? I tried to search it but I didn't find it.
Sorry, I forgot to said I found this in ObjectMemory >> markAndTrace:
(self isWeakNonInt: oop) ifTrue: [ "Set lastFieldOffset before the weak fields in the receiver" lastFieldOffset := (self nonWeakFieldsOf: oop) << ShiftForWord. "And remember as weak root" weakRootCount := weakRootCount + 1. weakRoots at: weakRootCount put: oop. ] ifFalse: [ "Do it the usual way" lastFieldOffset := self lastPointerOf: oop. ].
But I don't know if this make sense or not.
Thanks!!
Mariano
What you do with this piece of code:
symbolHolder := Symbol allSymbols.
is to hold those symbols there. So, when ImageSegment uses the GC techniques to detect which objects are ONLY pointed from inside of the segment, the symbols is not found (because it is accessible trough that test) and thus, it goes to outPointers instead of ByteArray.
And of course, if it is in outPointers instead of ByteArray when the segment is loaded again, yo don't create a symbol again but use the same object (the one of the oop).
I am correct? or I understood anything ?
yes.
Cheers, Adrian
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
On Wed, Dec 9, 2009 at 9:40 AM, Adrian Lienhard <adi@netstyle.ch> wrote:
I don't have the VMMaker code at hand right now (or rather, don't have time to look at it ;)), but I guess that after that piece of code you pasted below there is a loop from the first field (BaseHeader+4) to the last non-weak field (lastFieldOffset) in which the non-weak fields are traced. Note, that if an object both fixed and indexable weak fields the latter are always located after the former. You can see the memory layout and object formats on the pages 9 and 10 of the slides [1] (from a lecture about VMs I gave recently).
Thanks for the explanation Adrian and the link Adrian...now, being a bit off-topic, I have the following situation about ImageSegment. I tried to reproduce it in a unit test, but I couldn't yet :( Suppose I create a segment with a tree of objects. Suppose inside of that tree I have an object that refers to an external object called XXX, thus it is put in the outPointers. Then I extract the segment, I write it to disk, and I put a nil to the segment variable. After that, for some reason, my object XXX is garbage collected from my image. Then I want to load again my segment (reading the file and installling it). What happens ? it fails? If true, suppose I have already "installed" some other objects, what happen with those objects? is this atomic ? Thanks a lot for anyone that can give me a hint. Mariano
Cheers, Adrian
[1] http://www.adrian-lienhard.ch/presentations
On Dec 8, 2009, at 22:57 , Mariano Martinez Peck wrote:
On Tue, Dec 8, 2009 at 10:54 PM, Mariano Martinez Peck < marianopeck@gmail.com> wrote:
On Tue, Dec 8, 2009 at 5:07 PM, Adrian Lienhard <adi@netstyle.ch> wrote:
On Dec 8, 2009, at 12:08 , Mariano Martinez Peck wrote:
On Fri, Dec 4, 2009 at 12:05 PM, Adrian Lienhard <adi@netstyle.ch> wrote:
When you are creating your root of objects and you put symbols inside, they are not put in ourPointers but in ByteArray. This is due to the fact that the only object who is pointing to that symbol is inside the segment ?
To be precise, the symbols are also pointed to by the symbol table, but only by weak references. Since image segments use the GC mark logic, these pointers are not considered.
Ahhh ok. Now I see Symbol class >> rehash where it sets to SymbolTable := WeakSet
Now...my finally question is, where in the code you can see the GC only mark "normal" objects and that week objects are not being taken into account. Do you know ? I tried to search it but I didn't find it.
Sorry, I forgot to said I found this in ObjectMemory >> markAndTrace:
(self isWeakNonInt: oop) ifTrue: [ "Set lastFieldOffset before the weak fields in the receiver" lastFieldOffset := (self nonWeakFieldsOf: oop) << ShiftForWord. "And remember as weak root" weakRootCount := weakRootCount + 1. weakRoots at: weakRootCount put: oop. ] ifFalse: [ "Do it the usual way" lastFieldOffset := self lastPointerOf: oop. ].
But I don't know if this make sense or not.
Thanks!!
Mariano
What you do with this piece of code:
symbolHolder := Symbol allSymbols.
is to hold those symbols there. So, when ImageSegment uses the GC techniques to detect which objects are ONLY pointed from inside of the segment, the symbols is not found (because it is accessible trough that test) and thus, it goes to outPointers instead of ByteArray.
And of course, if it is in outPointers instead of ByteArray when the segment is loaded again, yo don't create a symbol again but use the same object (the one of the oop).
I am correct? or I understood anything ?
yes.
Cheers, Adrian
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
Hi Mariano, I don't understand your question(s)... If you have some code, that would probably help. Cheers, Adrian On Dec 9, 2009, at 21:06 , Mariano Martinez Peck wrote:
On Wed, Dec 9, 2009 at 9:40 AM, Adrian Lienhard <adi@netstyle.ch> wrote:
I don't have the VMMaker code at hand right now (or rather, don't have time to look at it ;)), but I guess that after that piece of code you pasted below there is a loop from the first field (BaseHeader+4) to the last non-weak field (lastFieldOffset) in which the non-weak fields are traced. Note, that if an object both fixed and indexable weak fields the latter are always located after the former. You can see the memory layout and object formats on the pages 9 and 10 of the slides [1] (from a lecture about VMs I gave recently).
Thanks for the explanation Adrian and the link Adrian...now, being a bit off-topic, I have the following situation about ImageSegment. I tried to reproduce it in a unit test, but I couldn't yet :(
Suppose I create a segment with a tree of objects. Suppose inside of that tree I have an object that refers to an external object called XXX, thus it is put in the outPointers. Then I extract the segment, I write it to disk, and I put a nil to the segment variable. After that, for some reason, my object XXX is garbage collected from my image. Then I want to load again my segment (reading the file and installling it). What happens ? it fails? If true, suppose I have already "installed" some other objects, what happen with those objects? is this atomic ?
Thanks a lot for anyone that can give me a hint.
Mariano
Cheers, Adrian
[1] http://www.adrian-lienhard.ch/presentations
On Dec 8, 2009, at 22:57 , Mariano Martinez Peck wrote:
On Tue, Dec 8, 2009 at 10:54 PM, Mariano Martinez Peck < marianopeck@gmail.com> wrote:
On Tue, Dec 8, 2009 at 5:07 PM, Adrian Lienhard <adi@netstyle.ch> wrote:
On Dec 8, 2009, at 12:08 , Mariano Martinez Peck wrote:
On Fri, Dec 4, 2009 at 12:05 PM, Adrian Lienhard <adi@netstyle.ch> wrote:
When you are creating your root of objects and you put symbols inside, they are not put in ourPointers but in ByteArray. This is due to the fact that the only object who is pointing to that symbol is inside the segment ?
To be precise, the symbols are also pointed to by the symbol table, but only by weak references. Since image segments use the GC mark logic, these pointers are not considered.
Ahhh ok. Now I see Symbol class >> rehash where it sets to SymbolTable := WeakSet
Now...my finally question is, where in the code you can see the GC only mark "normal" objects and that week objects are not being taken into account. Do you know ? I tried to search it but I didn't find it.
Sorry, I forgot to said I found this in ObjectMemory >> markAndTrace:
(self isWeakNonInt: oop) ifTrue: [ "Set lastFieldOffset before the weak fields in the receiver" lastFieldOffset := (self nonWeakFieldsOf: oop) << ShiftForWord. "And remember as weak root" weakRootCount := weakRootCount + 1. weakRoots at: weakRootCount put: oop. ] ifFalse: [ "Do it the usual way" lastFieldOffset := self lastPointerOf: oop. ].
But I don't know if this make sense or not.
Thanks!!
Mariano
What you do with this piece of code:
symbolHolder := Symbol allSymbols.
is to hold those symbols there. So, when ImageSegment uses the GC techniques to detect which objects are ONLY pointed from inside of the segment, the symbols is not found (because it is accessible trough that test) and thus, it goes to outPointers instead of ByteArray.
And of course, if it is in outPointers instead of ByteArray when the segment is loaded again, yo don't create a symbol again but use the same object (the one of the oop).
I am correct? or I understood anything ?
yes.
Cheers, Adrian
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo- project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
On Thu, Dec 10, 2009 at 9:21 AM, Adrian Lienhard <adi@netstyle.ch> wrote:
Hi Mariano,
I don't understand your question(s)... If you have some code, that would probably help.
Ok. I commit a test case called testInstallSegmenetWithObjectRemoved which is failing in the version Tests-Mariano.40 of the repository http://www.squeaksource.com/MarianoPhD Maybe what is happening is that "anExternalObject" is not being garbageCollected because there are references from the outPointers array. That sounds logic. What do you think? Kind regards, Mariano
Cheers, Adrian
On Dec 9, 2009, at 21:06 , Mariano Martinez Peck wrote:
On Wed, Dec 9, 2009 at 9:40 AM, Adrian Lienhard <adi@netstyle.ch> wrote:
I don't have the VMMaker code at hand right now (or rather, don't have time to look at it ;)), but I guess that after that piece of code you pasted below there is a loop from the first field (BaseHeader+4) to the last non-weak field (lastFieldOffset) in which the non-weak fields are traced. Note, that if an object both fixed and indexable weak fields the latter are always located after the former. You can see the memory layout and object formats on the pages 9 and 10 of the slides [1] (from a lecture about VMs I gave recently).
Thanks for the explanation Adrian and the link Adrian...now, being a bit off-topic, I have the following situation about ImageSegment. I tried to reproduce it in a unit test, but I couldn't yet :(
Suppose I create a segment with a tree of objects. Suppose inside of that tree I have an object that refers to an external object called XXX, thus it is put in the outPointers. Then I extract the segment, I write it to disk, and I put a nil to the segment variable. After that, for some reason, my object XXX is garbage collected from my image. Then I want to load again my segment (reading the file and installling it). What happens ? it fails? If true, suppose I have already "installed" some other objects, what happen with those objects? is this atomic ?
Thanks a lot for anyone that can give me a hint.
Mariano
Cheers, Adrian
[1] http://www.adrian-lienhard.ch/presentations
On Dec 8, 2009, at 22:57 , Mariano Martinez Peck wrote:
On Tue, Dec 8, 2009 at 10:54 PM, Mariano Martinez Peck < marianopeck@gmail.com> wrote:
On Tue, Dec 8, 2009 at 5:07 PM, Adrian Lienhard <adi@netstyle.ch> wrote:
On Dec 8, 2009, at 12:08 , Mariano Martinez Peck wrote:
On Fri, Dec 4, 2009 at 12:05 PM, Adrian Lienhard <adi@netstyle.ch> wrote:
When you are creating your root of objects and you put symbols inside, they are not put in ourPointers but in ByteArray. This is due to the fact that the only object who is pointing to that symbol is inside the segment ?
To be precise, the symbols are also pointed to by the symbol table, but only by weak references. Since image segments use the GC mark logic, these pointers are not considered.
Ahhh ok. Now I see Symbol class >> rehash where it sets to SymbolTable := WeakSet
Now...my finally question is, where in the code you can see the GC only mark "normal" objects and that week objects are not being taken into account. Do you know ? I tried to search it but I didn't find it.
Sorry, I forgot to said I found this in ObjectMemory >> markAndTrace:
(self isWeakNonInt: oop) ifTrue: [ "Set lastFieldOffset before the weak fields in the receiver" lastFieldOffset := (self nonWeakFieldsOf: oop) << ShiftForWord. "And remember as weak root" weakRootCount := weakRootCount + 1. weakRoots at: weakRootCount put: oop. ] ifFalse: [ "Do it the usual way" lastFieldOffset := self lastPointerOf: oop. ].
But I don't know if this make sense or not.
Thanks!!
Mariano
What you do with this piece of code:
symbolHolder := Symbol allSymbols.
is to hold those symbols there. So, when ImageSegment uses the GC techniques to detect which objects are ONLY pointed from inside of the segment, the symbols is not found (because it is accessible trough that test) and thus, it goes to outPointers instead of ByteArray.
And of course, if it is in outPointers instead of ByteArray when the segment is loaded again, yo don't create a symbol again but use the same object (the one of the oop).
I am correct? or I understood anything ?
yes.
Cheers, Adrian
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo- project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
Adrian Lienhard wrote:
On Dec 8, 2009, at 12:08 , Mariano Martinez Peck wrote:
On Fri, Dec 4, 2009 at 12:05 PM, Adrian Lienhard <adi@netstyle.ch> wrote:
When you are creating your root of objects and you put symbols inside, they are not put in ourPointers but in ByteArray. This is due to the fact that the only object who is pointing to that symbol is inside the segment ?
To be precise, the symbols are also pointed to by the symbol table, but only by weak references. Since image segments use the GC mark logic, these pointers are not considered.
Martin and I had been working towards the idea that ImageSegments should never create non-Canonical symbols, and the user should not have to use manual techniques to preserve symbols they are interested in keeping alive. I ended up getting stuck however, in trying to discover what process ImageSegment goes through while it is installing objects - ImageSegment #install has a call to an unimplemented method #readFromFileWithSymbols which it seems it should be using, but isn't....(wha' the?) install "This operation retrieves the segment if necessary from file storage, installs it in memory, and replaces (using become:) all the root stubs with the reconstructed roots of the segment." | newRoots | state = #onFile ifTrue: [self readFromFile]. state = #onFileWithSymbols ifTrue: [self readFromFileWithSymbols. endMarker := segment nextObject. "for enumeration of objects" endMarker == 0 ifTrue: [endMarker := 'End' clone]]. (state = #active) | (state = #imported) ifFalse: [self errorWrongState]. newRoots := self loadSegmentFrom: segment outPointers: outPointers. state = #imported ifTrue: ["just came in from exported file" arrayOfRoots := newRoots] ifFalse: [ arrayOfRoots elementsForwardIdentityTo: newRoots]. state := #inactive. Beeper beepPrimitive I think the next step in this trajectory is to trace is how the state is getting set - right now it is kind of a mystery... Does this seem reasonable? Thanks, Sheri -- View this message in context: http://n2.nabble.com/Re-getting-rid-of-Symbol-new-tp4109230p4143090.html Sent from the Pharo Smalltalk mailing list archive at Nabble.com.
Sheridan Mahoney wrote:
Martin and I had been working towards the idea that ImageSegments should never create non-Canonical symbols, and the user should not have to use manual techniques to preserve symbols they are interested in keeping alive.
I ended up getting stuck however, in trying to discover what process ImageSegment goes through while it is installing objects - ImageSegment #install has a call to an unimplemented method #readFromFileWithSymbols which it seems it should be using, but isn't....(wha' the?)
install "This operation retrieves the segment if necessary from file storage, installs it in memory, and replaces (using become:) all the root stubs with the reconstructed roots of the segment."
| newRoots | state = #onFile ifTrue: [self readFromFile]. state = #onFileWithSymbols ifTrue: [self readFromFileWithSymbols.
My guess (and only a guess) is that #onFileWithSymbols never becomes a state in the test case that's failing. And my opinion is that it's suspicious to require something different for a image segment that contains symbols as opposed to one that doesn't; the ideal would be to have image segments just transparently handle Symbols correctly. Users just shouldn't have to deal with details at that level; the correct semantics are extremely clear. Regards, -Martin
On Thu, Dec 10, 2009 at 3:32 AM, Martin McClure <martin@hand2mouse.com>wrote:
Sheridan Mahoney wrote:
Martin and I had been working towards the idea that ImageSegments should never create non-Canonical symbols, and the user should not have to use manual techniques to preserve symbols they are interested in keeping alive.
I ended up getting stuck however, in trying to discover what process ImageSegment goes through while it is installing objects - ImageSegment #install has a call to an unimplemented method #readFromFileWithSymbols which it seems it should be using, but isn't....(wha' the?)
install "This operation retrieves the segment if necessary from file storage, installs it in memory, and replaces (using become:) all the root stubs with the reconstructed roots of the segment."
| newRoots | state = #onFile ifTrue: [self readFromFile]. state = #onFileWithSymbols ifTrue: [self readFromFileWithSymbols.
My guess (and only a guess) is that #onFileWithSymbols never becomes a state in the test case that's failing.
Yes!
And my opinion is that it's suspicious to require something different for a image segment that contains symbols as opposed to one that doesn't; the ideal would be to have image segments just transparently handle Symbols correctly. Users just shouldn't have to deal with details at that level; the correct semantics are extremely clear.
142% agree. So...there are a couple of things I think ImageSegment should do instead of the user: 1) If you put in the segment a class, ImageSegment automatically puts also the metcaclass. 2) Deal transparently if you put symbols in the segment. 3) Evaluate "Smalltalk garbageCollect" after the message "extract". This is most of the cases something you want to do. I addition, look the senders of extract and you will see that in most of the cases, after doing the extract, it evaluates a garbageCollect. Cheers, Mariano
Regards,
-Martin
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
On Thu, Dec 10, 2009 at 3:32 AM, Martin McClure <martin@hand2mouse.com>wrote:
Sheridan Mahoney wrote:
Martin and I had been working towards the idea that ImageSegments should never create non-Canonical symbols, and the user should not have to use manual techniques to preserve symbols they are interested in keeping alive.
I ended up getting stuck however, in trying to discover what process ImageSegment goes through while it is installing objects - ImageSegment #install has a call to an unimplemented method #readFromFileWithSymbols which it seems it should be using, but isn't....(wha' the?)
install "This operation retrieves the segment if necessary from file storage, installs it in memory, and replaces (using become:) all the root stubs with the reconstructed roots of the segment."
| newRoots | state = #onFile ifTrue: [self readFromFile]. state = #onFileWithSymbols ifTrue: [self readFromFileWithSymbols.
My guess (and only a guess) is that #onFileWithSymbols never becomes a state in the test case that's failing.
Even more...#onFileWithSymbols becomes as state in the method writeToFileWithSymbols But, look the first line of this method: writeToFileWithSymbols | symbols nonSymbols pound | state = #extracted ifFalse: [self error: 'wrong state']. The state #extracted DOES NOT EXISTS!!! Look the senders and you will find only that method hahahahahah I guess it should be #active, because, as you can see in the method extract, the final states it sets is #active. So....you always get "wrong state" ;) Cheers, Mariano
And my opinion is that it's suspicious to require something different for a image segment that contains symbols as opposed to one that doesn't; the ideal would be to have image segments just transparently handle Symbols correctly. Users just shouldn't have to deal with details at that level; the correct semantics are extremely clear.
Regards,
-Martin
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
The objects in the segment's byte array are "instantiated" by the primitive #loadSegmentFrom:outPointers:. In fact they are not instantiated, but the byte array is just twisted a bit to represent real objects on the heap (internal references and indexes into outPointers are remapped). Adrian On Dec 10, 2009, at 01:59 , Sheridan Mahoney wrote:
Adrian Lienhard wrote:
On Dec 8, 2009, at 12:08 , Mariano Martinez Peck wrote:
On Fri, Dec 4, 2009 at 12:05 PM, Adrian Lienhard <adi@netstyle.ch> wrote:
When you are creating your root of objects and you put symbols inside, they are not put in ourPointers but in ByteArray. This is due to the fact that the only object who is pointing to that symbol is inside the segment ?
To be precise, the symbols are also pointed to by the symbol table, but only by weak references. Since image segments use the GC mark logic, these pointers are not considered.
Martin and I had been working towards the idea that ImageSegments should never create non-Canonical symbols, and the user should not have to use manual techniques to preserve symbols they are interested in keeping alive.
I ended up getting stuck however, in trying to discover what process ImageSegment goes through while it is installing objects - ImageSegment #install has a call to an unimplemented method #readFromFileWithSymbols which it seems it should be using, but isn't....(wha' the?)
install "This operation retrieves the segment if necessary from file storage, installs it in memory, and replaces (using become:) all the root stubs with the reconstructed roots of the segment."
| newRoots | state = #onFile ifTrue: [self readFromFile]. state = #onFileWithSymbols ifTrue: [self readFromFileWithSymbols. endMarker := segment nextObject. "for enumeration of objects" endMarker == 0 ifTrue: [endMarker := 'End' clone]]. (state = #active) | (state = #imported) ifFalse: [self errorWrongState]. newRoots := self loadSegmentFrom: segment outPointers: outPointers. state = #imported ifTrue: ["just came in from exported file" arrayOfRoots := newRoots] ifFalse: [ arrayOfRoots elementsForwardIdentityTo: newRoots]. state := #inactive. Beeper beepPrimitive
I think the next step in this trajectory is to trace is how the state is getting set - right now it is kind of a mystery...
Does this seem reasonable?
Thanks,
Sheri
-- View this message in context: http://n2.nabble.com/Re-getting-rid-of-Symbol-new-tp4109230p4143090.html Sent from the Pharo Smalltalk mailing list archive at Nabble.com.
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
Martin and I had been working towards the idea that ImageSegments should never create non-Canonical symbols, and the user should not have to use manual techniques to preserve symbols they are interested in keeping alive.
I ended up getting stuck however, in trying to discover what process ImageSegment goes through while it is installing objects - ImageSegment #install has a call to an unimplemented method #readFromFileWithSymbols which it seems it should be using, but isn't....(wha' the?)
Do you believe me if I told you that I checked in squeak 2.6 and the method (readFromFileWithSymbols) is still not implemented ? I think it never was hahahah
participants (8)
-
Adrian Lienhard -
Henrik Johansen -
James Foster -
John M McIntosh -
Mariano Martinez Peck -
Martin McClure -
Sheridan Mahoney -
Stéphane Ducasse