25 Sep
2013
25 Sep
'13
12:05 p.m.
Hi,
Yes, maybe we have missed something … because the Smalltalkimporter shows no performance
improvement.
When we look at the code and search for usages of "entityNamed:" we count a lot
of occurrences in the test, and one in the monticello importer.
So maybe a question that is more important is: do we need to be able to search on
mooseName fast? Or does it suffice if the storage only stores by type: since this is the
use case that is most apparent (allClasses, allTypes, etc.) and used. Importers need to
link, so they need to be able to use a dictionary for name lookup. But this lookup could
be private.
So our question is now:
- should we move our optimization to the monticello importer
and:
- throw out the by name.
- throw out the elements
Well, going to lunch now.
Cheers,
Diego
Hi Doru,
On Sep 25, 2013, at 6:02 AM, Tudor Girba wrote:
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
--
www.tudorgirba.com
"Every thing has its own flow"
_______________________________________________
Moose-dev mailing list
Moose-dev(a)iam.unibe.ch
https://www.iam.unibe.ch/mailman/listinfo/moose-dev
_______________________________________________
Moose-dev mailing list
Moose-dev(a)iam.unibe.ch
https://www.iam.unibe.ch/mailman/listinfo/moose-dev