When trying to walk smalltalkhub and import latest versions to find invocations, we needed to make the following changes to make it work:
FAMIXAccess>variable: aStructuralEntity (aStructuralEntity isKindOf: FAMIXAnnotationInstanceAttribute) ifFalse: [ variable := FMMultivalueLink on: self update: #incomingAccesses from: self variable to: aStructuralEntity]
Add MooseMonticelloVisitor>visitScriptDefinition: aMCPostscriptDefinition "TODO"
MooseMonticelloHTTPImporter>setRepositoryCache: aCache. repositoryCache := aCache
MooseMonticelloMethodPopulator>ensureAnnotationType: aRBPragmaNode ^self importer ensureAnnotationType: aRBPragmaNode.
And change for performance:
MooseGroupRuntimeStorage>add: anElement | key group |
key := anElement class. group := byType at: key ifAbsentPut: [ OrderedCollection new ]. group add: anElement. elements add: anElement. byName at: anElement mooseName asSymbol put: anElement. ^ anElement
MooseGroupRuntimeStorage >at: uniqueName ifAbsent: exceptionBlock | entity na | na := uniqueName asSymbol. "look first by name and if not found" entity := byName at: na ifAbsent: [nil]. entity notNil ifTrue: [ entity mooseName asSymbol = na ifTrue: [ ^entity] ifFalse: [ byName at: na put: nil. byName at: entity mooseName asSymbol put: entity. self halt. "Should this be happening?" ]]. ^ exceptionBlock value
Would they be safe to commit?
Hi,
On Wed, Sep 18, 2013 at 3:54 PM, Stephan Eggermont stephan@stack.nl wrote:
When trying to walk smalltalkhub and import latest versions to find invocations, we needed to make the following changes to make it work:
FAMIXAccess>variable: aStructuralEntity (aStructuralEntity isKindOf: FAMIXAnnotationInstanceAttribute) ifFalse: [ variable := FMMultivalueLink on: self update: #incomingAccesses from: self variable to: aStructuralEntity]
This is strange. Why do you need that?
Add
MooseMonticelloVisitor>visitScriptDefinition: aMCPostscriptDefinition "TODO"
MooseMonticelloHTTPImporter>setRepositoryCache: aCache. repositoryCache := aCache
MooseMonticelloMethodPopulator>ensureAnnotationType: aRBPragmaNode ^self importer ensureAnnotationType: aRBPragmaNode.
This looks Ok.
And change for performance:
MooseGroupRuntimeStorage>add: anElement | key group |
key := anElement class. group := byType at: key ifAbsentPut: [ OrderedCollection new ]. group add: anElement. elements add: anElement. byName at: anElement mooseName asSymbol put: anElement. ^ anElementMooseGroupRuntimeStorage >at: uniqueName ifAbsent: exceptionBlock
| entity na | na := uniqueName asSymbol. "look first by name and if not found" entity := byName at: na ifAbsent: [nil]. entity notNil ifTrue: [ entity mooseName asSymbol = na ifTrue: [ ^entity] ifFalse: [ byName at: na put: nil. byName at: entity mooseName asSymbol put: entity. self halt. "Should this be happening?" ]]. ^ exceptionBlock valueWould they be safe to commit?
This would not work well at the moment because MooseGroupRuntimeStorage is used also for all moose groups. We would need to have a storage that is less demanding for handling associations, and then we could rely on having name for everything.
Doru
Moose-dev mailing list Moose-dev@iam.unibe.ch https://www.iam.unibe.ch/mailman/listinfo/moose-dev
This would not work well at the moment because MooseGroupRuntimeStorage is used also for all moose groups. We would need to have a storage that is less demanding for handling associations, and then we could rely on having name for everything.
Doru
Hi Doru,
Most elements we have a unique name, and we should be able to find them (fast) by using the lookup. This is what "at: ifAbsent:" implies. I don't think this method can be used on an entity that does not have a unique name. There is only one problem: 1) either we assert that a change in the moose name is not possible. This means that if we have a cache miss, we should execute the exception block. 2) Or we assume that a change in the moose name is possible. This means that we have to check if we have a cache hit, and have to go through all elements if we have a cache miss.
Currently the implementation is a bit heuristic, returning if we have a cache hit, and adding when we go through all elements. My problem is that the tests rely on this "mixed" behavior. So … I do not understand why it was built this way.
When I look at the implementation of MooseEntity mooseName, it uses a privateState to cache the mooseName, but it returns before it actually stores it. And removing the return statement causes a lot of tests to fail. So this means that the Moose name changes …
Diego
Man, this is messed up :).
The mooseName should better be cached as it is not expected to change. The tests fail precisely because it is the tests that want to go around these issue and introduce namespaces artificially after the actual import. Take a look at:
FAMIXNavigationPluginTestsResource>>importModel
But, for associations we have no benefit of caching them by name because we never look them up by name. That is why in the case of association we should use a different Storage type that does not build a cache.
Cheers, Doru
On Wed, Sep 18, 2013 at 5:05 PM, Diego Lont diego.lont@delware.nl wrote:
This would not work well at the moment because MooseGroupRuntimeStorage
is used also for all moose groups. We would need to have a storage that is less demanding for handling associations, and then we could rely on having name for everything.
Doru
Hi Doru,
Most elements we have a unique name, and we should be able to find them (fast) by using the lookup. This is what "at: ifAbsent:" implies. I don't think this method can be used on an entity that does not have a unique name. There is only one problem: 1) either we assert that a change in the moose name is not possible. This means that if we have a cache miss, we should execute the exception block. 2) Or we assume that a change in the moose name is possible. This means that we have to check if we have a cache hit, and have to go through all elements if we have a cache miss.
Currently the implementation is a bit heuristic, returning if we have a cache hit, and adding when we go through all elements. My problem is that the tests rely on this "mixed" behavior. So … I do not understand why it was built this way.
When I look at the implementation of MooseEntity mooseName, it uses a privateState to cache the mooseName, but it returns before it actually stores it. And removing the return statement causes a lot of tests to fail. So this means that the Moose name changes …
Diego
Moose-dev mailing list Moose-dev@iam.unibe.ch https://www.iam.unibe.ch/mailman/listinfo/moose-dev
Yes, you can say that.
So I looked at the tests and what it all uses. And together with Stephan I made an implementation that at least holds up all tests. Of course, we have rewritten the FAMIXNavigationPluginTestsResource.
The specifics: > we have introduced a method that says wether it has a unique moose name, and therefor should be cached by name and be updated if it is out of sync. It is set to true for a namedEntity and for a MooseFile.
> we have created the possibility to clear the moose name. Note there are two methods: one that also updates the model, and one that only clears the moose name. The method resetMooseName should be called if appropriate. Do not call the method that only clears the moose name, since this method destroys the cache.
> the moose name is now cached in a moose entity, and therefor will only change if the "clearMooseName" method is called. This method should be called if: 1. The object returns true on "hasUniqueMooseNameInModel" 2. The mooseName changes because of a change. I.E. The implementation of setting the signature of a behavioralEntity should be: signature: aString signature := aString. self resetMooseName
> the implementation of resetMooseName should call all its children that depend on the moose name of this entity, to also reset their moose name. (so this is expensive)
> The lookup is now a dictionary lookup, that returns immediately. I probably should add a guard that checks if the object already exists, but I don't want to change too much at the same time, so I'll leave it for now.
Important notes: > This implementation makes it possible to rename things AFTER they have been added to the model. This is however rather expensive. So if possible, one should avoid renaming and reorganizing things after they have been added to the model. > This causes the lookup to be cheap. A entityByName: is relative cheap. This was rather expensive, but now no longer is.
All tests we have run (all famix tests in moose, including java, etc. ) are green now, so I will check this in.
Cheers, Stephan and Diego
On Sep 18, 2013, at 9:40 PM, Tudor Girba wrote:
Man, this is messed up :).
The mooseName should better be cached as it is not expected to change. The tests fail precisely because it is the tests that want to go around these issue and introduce namespaces artificially after the actual import. Take a look at:
FAMIXNavigationPluginTestsResource>>importModel
But, for associations we have no benefit of caching them by name because we never look them up by name. That is why in the case of association we should use a different Storage type that does not build a cache.
Cheers, Doru
On Wed, Sep 18, 2013 at 5:05 PM, Diego Lont diego.lont@delware.nl wrote:
This would not work well at the moment because MooseGroupRuntimeStorage is used also for all moose groups. We would need to have a storage that is less demanding for handling associations, and then we could rely on having name for everything.
Doru
Hi Doru,
Most elements we have a unique name, and we should be able to find them (fast) by using the lookup. This is what "at: ifAbsent:" implies. I don't think this method can be used on an entity that does not have a unique name. There is only one problem: 1) either we assert that a change in the moose name is not possible. This means that if we have a cache miss, we should execute the exception block. 2) Or we assume that a change in the moose name is possible. This means that we have to check if we have a cache hit, and have to go through all elements if we have a cache miss.
Currently the implementation is a bit heuristic, returning if we have a cache hit, and adding when we go through all elements. My problem is that the tests rely on this "mixed" behavior. So … I do not understand why it was built this way.
When I look at the implementation of MooseEntity mooseName, it uses a privateState to cache the mooseName, but it returns before it actually stores it. And removing the return statement causes a lot of tests to fail. So this means that the Moose name changes …
Diego
Moose-dev mailing list Moose-dev@iam.unibe.ch https://www.iam.unibe.ch/mailman/listinfo/moose-dev
-- www.tudorgirba.com
"Every thing has its own flow" _______________________________________________ Moose-dev mailing list Moose-dev@iam.unibe.ch https://www.iam.unibe.ch/mailman/listinfo/moose-dev
-
Diego Lont -
Stephan Eggermont -
Tudor Girba