Changes to Famix-Core, Moose-Core & Moose-MonticelloImporter safe?

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

Changes to Famix-Core, Moose-Core & Moose-MonticelloImporter safe?

Stephan Eggermont-3
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?
_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev
Reply | Threaded
Open this post in threaded view
|

Re: Changes to Famix-Core, Moose-Core & Moose-MonticelloImporter safe?

Tudor Girba-2
Hi,


On Wed, Sep 18, 2013 at 3:54 PM, Stephan Eggermont <[hidden email]> 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.
        ^ 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?

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
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev



--

"Every thing has its own flow"

_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev
Reply | Threaded
Open this post in threaded view
|

Re: Changes to Famix-Core, Moose-Core & Moose-MonticelloImporter safe?

Stephan Eggermont-3
In reply to this post by Stephan Eggermont-3
I wrote:
>FAMIXAccess>variable: aStructuralEntity
>        (aStructuralEntity isKindOf: FAMIXAnnotationInstanceAttribute) ifFalse: [
>       variable := FMMultivalueLink on: self
>                                        update: #incomingAccesses
>                                        from: self variable
>                                        to: aStructuralEntity]

Dory wrote:
>This is strange. Why do you need that?

We tried to do

findDeprecationsFor: aVersion in: aProject
        | mooseModel |
        mooseModel := (MooseMonticelloHTTPImporter new setRepositoryCache: aProject monticelloRepository;
                importFileNamed: (aVersion at: #filename)).
        mooseModel allInvocations do: [ :each|
                aProject addInvocation: each inVersion: aVersion ]



_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev
Reply | Threaded
Open this post in threaded view
|

Re: Changes to Famix-Core, Moose-Core & Moose-MonticelloImporter safe?

DiegoLont
In reply to this post by Tudor Girba-2
>
> 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
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev
Reply | Threaded
Open this post in threaded view
|

Re: Changes to Famix-Core, Moose-Core & Moose-MonticelloImporter safe?

Tudor Girba-2
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 <[hidden email]> 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
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev



--

"Every thing has its own flow"

_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev
Reply | Threaded
Open this post in threaded view
|

Re: Changes to Famix-Core, Moose-Core & Moose-MonticelloImporter safe?

DiegoLont
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 <[hidden email]> 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
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev



--

"Every thing has its own flow"
_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev


_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev