On 27/11/2016 11:15, Peter Levart wrote:
Hi Alan,
Overall this looks very good. I noticed a couple of nits...
Thanks for going through the changes.
:
So should Provider<S> rather declare the following?
Class<? extends S> type();
Or alternatively, should ServiceLoader rather declare the following?
public Stream<? extends Provider<? extends S>> stream();
Well spotted, this should be Class<? extends S>. We could change the
return of stream too but that might be more awkward to work with.
:
Now that there can be multiple parents of some configuration, those
parents can have common parents, but since you iterate the transitive
configurations of each individual parent in isolation, you can
encounter a common "diamond" configuration multiple times. Logically
all is well since you collect the required modules into Sets so
multiplicity is taken care of. But this is not very optimal from
algorithm point. It would be better to 1st make a stream of unique
configurations of all parents:
Fair comment, we haven't done any performance work for the multi parent
scenario yet, mostly because #NonHierarchicalLayers is niche and there
are still some open questions on how parents should be searched.
So we can change this as you suggest except that it regresses startup,
this is the reason for skipping Configuration.empty(). However we
support both so I'll get that change in so that the more elegant code is
used when creating dynamic configurations.
-Alan
