+1 to modular and questioning non-constant use of static

> On 3 Apr, 2017, at 09:27, Anthony Baker <aba...@pivotal.io> wrote:
> 
> 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?
>>>>>> 
>>>>>>> 
>>>>> 
>>> 
>> 
> 

Reply via email to