Hi,
I would like to signal that Diego and Stephan worked on some internal changes to Moose-Core that can have impact on other code. So, here is a little review to start the discussion:
- mooseName is now cached. This can have an impact if you rely on your model to be more dynamic. If you have a custom entity that can receive sub-entities at runtime, you might want to resetMooseName. For example, if you modify the scope of a FAMIXType, the mooseName has to be reset:
FAMIXType>>container: aContainerEntity
container := FMMultivalueLink on: self
update: #types
from: self container
to: aContainerEntity.
self resetMooseName
This is supposed to be an optimization, but it makes the code more complicated. To see if it is actually worth it, we should benchmark the impact.
@Diego, @Stephan: Did you do some benchmarking on this?
We did not run an official benchmark, but we used Moose to make a deprecation finder, to look at all recent code in Smalltalk hub if there are calls to deprecated methods. Actually it finds all calls made and stores them all, and we do not link them to deprecated methods yet, but the work of this last step is to build a list of deprecated methods. The deprecation finder uses a MonticelloImporter to import a single monticello packages into a new Moose model and then stores the signatures of the invocations.
Doing this, we experienced that this was rather slow, so we dived into the code to search for places we could optimize. We noted that especially the larger packages suffered from the lookup "entityAt:" to build the calls. It searched linear through all entities (about 64.000), to create the candidates for an invocation. Since there are plenty of invocations in a normal sized package, this took rather long.
After revising the code we had to build in a delay, not to bring smalltalkhub down. And with the delay (of 500 ms per monticello package) it is still faster then it used to be. So I think this proves the performance upgrade enough. But if you have suggestions for a good benchmark, we are willing to run this. We do not think the tests provide a good benchmark, since these contain small models and test for atypical use (renames and such). Maybe it is an idea to use running moose on moose as benchmark?
In the comments of the mooseName, it already states that the mooseName should not change. However we found out that plenty of code (especially tests) relied on the fact that it could change, so we decided to build in a failsafe. Yes, it makes the code more complicated, but it allows to set properties, that causes the mooseName to change, after it is added to the model (like the name and container).
- An entity now answers if it hasUniqueMooseNameInModel. This is at the moment used when caching the entity by name in groups. For example, Associations do not have a unique name and thus, they are not cached.
- There are some parts of the code I do not understand. For example, the need of this code:
MooseGroupStorage>>becomeKind: elementStorageClass
...
self do: [ :each | each hasUniqueMooseNameInModel ifTrue: [ each privateClearMooseName ] ].
…
This might be an old code snippet, that was needed to reset the cache. Later we made it possible to reset the cache more precise, so it is probably redundant now. We should test if we can remove it now. It makes sure that when you go from own type of storage to another type, all Moose names are recalculated.
Did I miss anything?
Cheers,
Doru
--
"Every thing has its own flow"
Moose-dev mailing list
Moose-dev@iam.unibe.ch
https://www.iam.unibe.ch/mailman/listinfo/moose-dev