+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?
>>>>>>
>>>>>>>
>>>>>
>>>
>>
>