How to tell if a class still needs a pool dictionary

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

How to tell if a class still needs a pool dictionary

Louis LaBrunda
Hi All,

I have done some refactoring that I think has removed the need for a pool dictionary.  Is there a clean way to tell if a pool dictionary is needed without just removing it and putting it back if it turns out it is still needed?

Lou

--
You received this message because you are subscribed to the Google Groups "VAST Community Forum" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/va-smalltalk/67607f11-c775-468b-b329-c67d1d70b2b1n%40googlegroups.com.
Reply | Threaded
Open this post in threaded view
|

Re: How to tell if a class still needs a pool dictionary

Richard Sargent
Administrator
On Thursday, April 15, 2021 at 7:51:41 AM UTC-7 [hidden email] wrote:
Hi All,

I have done some refactoring that I think has removed the need for a pool dictionary.  Is there a clean way to tell if a pool dictionary is needed without just removing it and putting it back if it turns out it is still needed?

I'm not aware of such a tool, but I don't think it would be difficult to create.
Take the associations in the pool dictionary and add them to a identity set.
Walk all the classes (see something like "class hierarchy roots do:") and examine all the methods in the class and metaclass method dictionaries. Examine each method's literals for Associations and test whether the identity set includes any you find.

After that, it's just a matter of book keeping for what you want to do / report.


Lou

--
You received this message because you are subscribed to the Google Groups "VAST Community Forum" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/va-smalltalk/7f893c32-c9be-46ed-9e1d-7435728f5c44n%40googlegroups.com.
Reply | Threaded
Open this post in threaded view
|

Re: How to tell if a class still needs a pool dictionary

Richard Sargent
Administrator
Here. Try this. Tweak as needed.

(Blasted web programmers. "Error posting message". Thanks, that's sooo helpful.)
Since I cannot attach the script, I'll paste it.

| pool associations references report |
pool := CldtConstants.

asssociations := IdentitySet withAll: pool associations.    "EsPoolAssociation"
references := IdentityDictionary new.
report := WriteStream on: String new.
System classHierarchyRoots do: [:cl |
    cl allMethodsDo: [:cm |
        cm allLiteralsDo: [:lit |
            lit epIsPoolAssociation ifTrue: [
                (asssociations includes: lit) ifTrue: [
                    (references at: lit ifAbsentPut: [OrderedCollection new]) add: cm.
                ]
            ]
        ]
    ]
].

references isEmpty ifTrue: [^'Pool dictionary is unreferenced.'].
report nextPutAll: 'Pool Variable    Referencing Methods'.
(references keys asSortedCollection: [:a :b | a key < b key]) do: [:assoc |
    | refMethods |
    refMethods := (references at: assoc) asSortedCollection: CompiledMethod sortBlock.
    report cr; nextPutAll: assoc key.
    refMethods do: [:cm | report tab; print: cm].
].
report contents

<img src="data:;base64,fCBwb29sIGFzc29jaWF0aW9ucyByZWZlcmVuY2VzIHJlcG9ydCB8DQpwb29sIDo9IENsZHRDb25zdGFudHMuDQoNCmFzc3NvY2lhdGlvbnMgOj0gSWRlbnRpdHlTZXQgd2l0aEFsbDogcG9vbCBhc3NvY2lhdGlvbnMuCSJFc1Bvb2xBc3NvY2lhdGlvbiINCnJlZmVyZW5jZXMgOj0gSWRlbnRpdHlEaWN0aW9uYXJ5IG5ldy4NCnJlcG9ydCA6PSBXcml0ZVN0cmVhbSBvbjogU3RyaW5nIG5ldy4NClN5c3RlbSBjbGFzc0hpZXJhcmNoeVJvb3RzIGRvOiBbOmNsIHwNCgljbCBhbGxNZXRob2RzRG86IFs6Y20gfA0KCQljbSBhbGxMaXRlcmFsc0RvOiBbOmxpdCB8DQoJCQlsaXQgZXBJc1Bvb2xBc3NvY2lhdGlvbiBpZlRydWU6IFsNCgkJCQkoYXNzc29jaWF0aW9ucyBpbmNsdWRlczogbGl0KSBpZlRydWU6IFsNCgkJCQkJKHJlZmVyZW5jZXMgYXQ6IGxpdCBpZkFic2VudFB1dDogW09yZGVyZWRDb2xsZWN0aW9uIG5ld10pIGFkZDogY20uDQoJCQkJXQ0KCQkJXQ0KCQldDQoJXQ0KXS4NCg0KcmVmZXJlbmNlcyBpc0VtcHR5IGlmVHJ1ZTogW14nUG9vbCBkaWN0aW9uYXJ5IGlzIHVucmVmZXJlbmNlZC4nXS4NCnJlcG9ydCBuZXh0UHV0QWxsOiAnUG9vbCBWYXJpYWJsZQlSZWZlcmVuY2luZyBNZXRob2RzJy4NCihyZWZlcmVuY2VzIGtleXMgYXNTb3J0ZWRDb2xsZWN0aW9uOiBbOmEgOmIgfCBhIGtleSA8IGIga2V5XSkgZG86IFs6YXNzb2MgfA0KCXwgcmVmTWV0aG9kcyB8DQoJcmVmTWV0aG9kcyA6PSAocmVmZXJlbmNlcyBhdDogYXNzb2MpIGFzU29ydGVkQ29sbGVjdGlvbjogQ29tcGlsZWRNZXRob2Qgc29ydEJsb2NrLg0KCXJlcG9ydCBjcjsgbmV4dFB1dEFsbDogYXNzb2Mga2V5Lg0KCXJlZk1ldGhvZHMgZG86IFs6Y20gfCByZXBvcnQgdGFiOyBwcmludDogY21dLg0KXS4NCnJlcG9ydCBjb250ZW50cw==" alt="">

--
You received this message because you are subscribed to the Google Groups "VAST Community Forum" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/va-smalltalk/15cd532e-cc02-4517-87a8-f3e975a4e713n%40googlegroups.com.
Reply | Threaded
Open this post in threaded view
|

Re: How to tell if a class still needs a pool dictionary

Noschvie
Hi Lou
have a look to the config map "z.ST: ENVY/QA" named "Smalltalk Quality Assurance Tools".
The suite includes a static testing tool called "Code Critic". The Code Critic
analyzes methods, classes, applications, and ENVY/Developer configuration maps
for common coding problems, using 37 built-in reviews.


--
You received this message because you are subscribed to the Google Groups "VAST Community Forum" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/va-smalltalk/7cfdda5b-2309-42d9-b61f-b1813b6f85b4n%40googlegroups.com.
Reply | Threaded
Open this post in threaded view
|

Re: How to tell if a class still needs a pool dictionary

Louis LaBrunda
Hi Guys,

Thanks for the posts.  It seems this test is not needed.  I started working on an extension the #SubApplication, thinking I could send a message to an application and have it check all its classes to see if they really needed their defined pool dictionaries.  While testing I discovered that all the methods don't really get complied if there is a problem with removing a pool dictionary.  You are warned there is a problem.

I'm working with V10.0.0.  I'm not sure but I think this is a new behavior and I'm not sure when it changed but I like it.  Thanks to whom ever came up with it.  If it is new, someone please let me know so I don't think I'm crazy for thinking it use to not work this way.

Lou

On Friday, April 16, 2021 at 2:54:54 AM UTC-4 Norbert Schlemmer wrote:
Hi Lou
have a look to the config map "z.ST: ENVY/QA" named "Smalltalk Quality Assurance Tools".
The suite includes a static testing tool called "Code Critic". The Code Critic
analyzes methods, classes, applications, and ENVY/Developer configuration maps
for common coding problems, using 37 built-in reviews.


--
You received this message because you are subscribed to the Google Groups "VAST Community Forum" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/va-smalltalk/b8eb043c-00a7-4e05-830e-05307280d771n%40googlegroups.com.
Reply | Threaded
Open this post in threaded view
|

Re: How to tell if a class still needs a pool dictionary

Seth Berman
Hi Lou,

Where is the warning you are seeing?  In the transcript?  In the editor?

- Seth

On Saturday, April 17, 2021 at 10:32:08 AM UTC-4 [hidden email] wrote:
Hi Guys,

Thanks for the posts.  It seems this test is not needed.  I started working on an extension the #SubApplication, thinking I could send a message to an application and have it check all its classes to see if they really needed their defined pool dictionaries.  While testing I discovered that all the methods don't really get complied if there is a problem with removing a pool dictionary.  You are warned there is a problem.

I'm working with V10.0.0.  I'm not sure but I think this is a new behavior and I'm not sure when it changed but I like it.  Thanks to whom ever came up with it.  If it is new, someone please let me know so I don't think I'm crazy for thinking it use to not work this way.

Lou

On Friday, April 16, 2021 at 2:54:54 AM UTC-4 Norbert Schlemmer wrote:
Hi Lou
have a look to the config map "z.ST: ENVY/QA" named "Smalltalk Quality Assurance Tools".
The suite includes a static testing tool called "Code Critic". The Code Critic
analyzes methods, classes, applications, and ENVY/Developer configuration maps
for common coding problems, using 37 built-in reviews.


--
You received this message because you are subscribed to the Google Groups "VAST Community Forum" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/va-smalltalk/36f7d673-948d-48c0-867c-30250aa15d19n%40googlegroups.com.
Reply | Threaded
Open this post in threaded view
|

Re: How to tell if a class still needs a pool dictionary

Louis LaBrunda
Hi Seth,

It is in the editor in the "Class Definition" tab where the ... poolDictionaries: ... are listed.  If you remove one you shouldn't and tell it to #save from the popup menu, it will not save and put squiggly lines under the pool dictionary name.  I followed the code a little and it talks about doing a virtual compile to catch this and other compile errors without doing real damage.

Lou

On Saturday, April 17, 2021 at 10:36:17 AM UTC-4 Seth Berman wrote:
Hi Lou,

Where is the warning you are seeing?  In the transcript?  In the editor?

- Seth

On Saturday, April 17, 2021 at 10:32:08 AM UTC-4 [hidden email] wrote:
Hi Guys,

Thanks for the posts.  It seems this test is not needed.  I started working on an extension the #SubApplication, thinking I could send a message to an application and have it check all its classes to see if they really needed their defined pool dictionaries.  While testing I discovered that all the methods don't really get complied if there is a problem with removing a pool dictionary.  You are warned there is a problem.

I'm working with V10.0.0.  I'm not sure but I think this is a new behavior and I'm not sure when it changed but I like it.  Thanks to whom ever came up with it.  If it is new, someone please let me know so I don't think I'm crazy for thinking it use to not work this way.

Lou

On Friday, April 16, 2021 at 2:54:54 AM UTC-4 Norbert Schlemmer wrote:
Hi Lou
have a look to the config map "z.ST: ENVY/QA" named "Smalltalk Quality Assurance Tools".
The suite includes a static testing tool called "Code Critic". The Code Critic
analyzes methods, classes, applications, and ENVY/Developer configuration maps
for common coding problems, using 37 built-in reviews.


--
You received this message because you are subscribed to the Google Groups "VAST Community Forum" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/va-smalltalk/a65cdc67-0c18-4da3-b82d-6a4d2389c9b3n%40googlegroups.com.
Reply | Threaded
Open this post in threaded view
|

Re: How to tell if a class still needs a pool dictionary

Seth Berman
Hi Lou,

Yes, that is the EtClassDefinitionCompiler hooked into the browsers.

It actually received some pretty big updates in both 10.0.0 and 10.0.1 to make it more friendly, as it does impose constraints which are either appreciated or not:)
I've had support cases involving both sentiments.  However, I really paid attention to the negative ones and tried to make some improvements so its not too draconian when folks are attempting to refactor in the editor in an order in which they prefer.  There were a lot of situations before 10.0.1 that produced a sort of "chicken-and-egg" problem when you were trying to refactor.

There is actually an interesting bit of history here, which some folks might like more information on, regarding why we felt the need to develop this.  I'll give some background, and then conclude with what the latest updates do.

First, why was this feature developed?
When performing a refactoring in manual steps in the browser, as opposed to an automated workflow like the various refactoring browser actions, the onus is on developer to refactor the code from one valid state to another.  The refactoring tools are responsible for doing this as one logically atomic transition.  Developers in the browser must do this in a sequence of manual steps.  During this time, the methods and class definition are often out of sync with each-other.  Unlike the way a lot of other language implementations and environments work, the browsers (used to) allow the class definition to be shaped separately from the compiled methods which may refer to it.  Even worse, the bytecodes are cached, so there are minimal opportunities for recompilation from source to recognize that the class definition and a given method are out of sync.

The most common situation we've seen (and experienced) in the past is the removal of instance variables, but methods remain that still refer to it.  Methods that refer to it have cached bytecodes that don't see it as a name, but rather an offset into the object.  When this code is executed, any number of outcomes can occur that make it extremely time consuming to track down.  In the best case, your running program will all the sudden inexplicably GPF.  In the worst case, the bytecode instvar offset now refers to "something else".  It could be another instvar or some bytes from an adjacent object which just happens to look like a valid object (i.e. tagged small integer).  And your program may continue to run for some time until you have all the wrong answers.  Even worse, you export your code (along with your cached bytecodes) to someone else...maybe it even makes it into your build system.  We've seen all of this, both because we are imperfect ourselves and have performed incomplete refactorings, but also because it used to fall in our laps as support cases.  And with so little traceability back to the root cause, it cost everyone involved a lot of wasted energy and time.

In a perfect world, we as developers are all perfect and perform these refactorings exactly as we intended.  But it isn't true, I have the support cases to prove it and my own personal experiences to match them.  If the penalty for getting it wrong were not so high, we might have been content to just let it go...but it was becoming very costly.  And the decision was made to develop the EtClassDefinitionCompiler in the hopes of squashing these issues for everybody.

Since that time we have not had a support case involving this issue.  On the other hand, it does impose constraints.  Some were unnecessary, and thanks to some support cases, I believe we have improved that situation in the latest releases.  But lets be honest, we're a community of folks that back a highly flexible and reflective environment with a very dynamic type system...constraints are not something that we appreciate:)  The very first thing that went into the EtClassDefinitionCompiler was the Enabled (true/false) class variable to just disable it...though hopefully people might now understand why I was reluctant to broadcast that.

10.0.0 - Turn it off for some time
Understandably, there were just some situations coming up where it was too constraining.  I received some support cases to this effect.  Of course, I offered them the way to disable it, but I thought I would do one better and integrate it with the browsers.  If you see the red squiggle come up in the class definition browser, but feel strongly you must continue forward in the order you wish, now you can hover over the squiggle which brings up a calltip with an up/down arrow.  Press the down arrow and it will allow you to disable the class definition browser for a period of time or until image restart.

10.0.1 - Consider proposed class definition
Thanks to a support case, an area was missed that was making things more constraining then it should have been.  Some folks were trying to change the superclass in the editor only to find that it wouldn't let them save it, but should have been valid to do so.  This was because the class definition compiler only considered the shape of the current class and its current hierarchy, not the shape that would be true if the class was reparented to what was being requested.  Now it looks at the shape of the proposed superclass.

I hope this was informative and provided some interesting background.  Have a great rest of the weekend!

On Saturday, April 17, 2021 at 11:01:31 AM UTC-4 [hidden email] wrote:
Hi Seth,

It is in the editor in the "Class Definition" tab where the ... poolDictionaries: ... are listed.  If you remove one you shouldn't and tell it to #save from the popup menu, it will not save and put squiggly lines under the pool dictionary name.  I followed the code a little and it talks about doing a virtual compile to catch this and other compile errors without doing real damage.

Lou

On Saturday, April 17, 2021 at 10:36:17 AM UTC-4 Seth Berman wrote:
Hi Lou,

Where is the warning you are seeing?  In the transcript?  In the editor?

- Seth

On Saturday, April 17, 2021 at 10:32:08 AM UTC-4 [hidden email] wrote:
Hi Guys,

Thanks for the posts.  It seems this test is not needed.  I started working on an extension the #SubApplication, thinking I could send a message to an application and have it check all its classes to see if they really needed their defined pool dictionaries.  While testing I discovered that all the methods don't really get complied if there is a problem with removing a pool dictionary.  You are warned there is a problem.

I'm working with V10.0.0.  I'm not sure but I think this is a new behavior and I'm not sure when it changed but I like it.  Thanks to whom ever came up with it.  If it is new, someone please let me know so I don't think I'm crazy for thinking it use to not work this way.

Lou

On Friday, April 16, 2021 at 2:54:54 AM UTC-4 Norbert Schlemmer wrote:
Hi Lou
have a look to the config map "z.ST: ENVY/QA" named "Smalltalk Quality Assurance Tools".
The suite includes a static testing tool called "Code Critic". The Code Critic
analyzes methods, classes, applications, and ENVY/Developer configuration maps
for common coding problems, using 37 built-in reviews.


--
You received this message because you are subscribed to the Google Groups "VAST Community Forum" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/va-smalltalk/6ce6bc07-4ebc-44c2-bb33-637ed5367282n%40googlegroups.com.