Using singletons leads to very monolithic systems that are hard to test and hard to change. Instead we should prefer modular services like Udo proposed.
I would go further and say that we should question any non-constant use of “static”. Anthony > On Apr 3, 2017, at 9:01 AM, Udo Kohlmeyer <ukohlme...@pivotal.io> wrote: > > Correct, that would be the definition. > > Yet, we find that our use of singletons within Geode is limiting to say that > least. With the idea of wanting to be able to create/run multiple cache > instance within the same JVM (especially for testing) a singleton will be > problematic. > > In addition to that, the alternative is not that hard to construct and in > many cases easier to manage. > > --Udo > > > On 4/3/17 08:57, Jinmei Liao wrote: >> Isn't "that instance is reused each invocation" my understanding of a >> "singleton"? >> >> On Mon, Apr 3, 2017 at 11:49 AM, Udo Kohlmeyer <ukohlme...@pivotal.io> >> wrote: >> >>> -1 For using singletons >>> >>> Using a Factory pattern you can avoid having to create singletons in >>> addition to caching created commands to avoid the recreation of the >>> instance. >>> >>> The SSLConfigurationFactory is a simple example where you create an >>> instance when required. Once an instance is created, that instance is >>> reused each invocation. >>> >>> --Udo >>> >>> >>> >>> On 4/3/17 08:30, Jinmei Liao wrote: >>> >>>> I think the client commands needs to be singleton instances even after you >>>> change the sequence of initialization. We don't want to have each client >>>> operation ends up creating a new command instance, right? That would be a >>>> more performance drag. >>>> >>>> On Thu, Mar 30, 2017 at 2:14 PM, Kirk Lund <kl...@apache.org> wrote: >>>> >>>> PS: I'll be writing and using JMH benchmarks to drive these changes. I'll >>>>> also create new unit tests for each of these classes that don't currently >>>>> have unit tests. >>>>> >>>>> On Thu, Mar 30, 2017 at 10:58 AM, Kirk Lund <kl...@apache.org> wrote: >>>>> >>>>> The client Commands now check with SecurityService even when security is >>>>>> not configured. This has introduced a negative performance impact. >>>>>> >>>>>> The best way to fix something like this is to tell the Command instance >>>>>> when it's being constructed that is does or does not need to perform >>>>>> security checks. >>>>>> >>>>>> Unfortunately, Commands are all implemented as singletons which are very >>>>>> eagerly instantiated during class loading of CommandInitializer (who >>>>>> thought that was a good idea?!). >>>>>> >>>>>> In order to fix this performance problem, I would need to get rid of >>>>>> >>>>> these >>>>> >>>>>> problematic static initializer blocks that so eagerly construct the >>>>>> Commands so that I can put off constructing them until AFTER the Cache >>>>>> is >>>>>> initializing and specifically AFTER the Cache has determined if it is >>>>>> >>>>> using >>>>> >>>>>> security or not. >>>>>> >>>>>> This means I'm going to have to do some refactoring of >>>>>> >>>>> CommandInitializer, >>>>> >>>>>> the Command classes, ServerConnection, AcceptorImpl, etc. >>>>>> >>>>>> Any other approach is going to have such minimal impact on performance >>>>>> that I'm not even interested in doing less than this. >>>>>> >>>>>> From a very high level, I would change the code so that the Cache owns >>>>>> >>>>> the >>>>> >>>>>> Server which owns the Command instances. In this way, the configuring of >>>>>> use of security can trickle down from Cache to each Command. I would >>>>>> primarily be altering static singletons, static initializers and adding >>>>>> constructors. >>>>>> >>>>>> Does anyone have a problem with me changing the above classes and >>>>>> especially getting rid of the static initializers and singleton >>>>>> >>>>> instances? >>>>> >>>>>> >>>> >> >
