Re: SHINDIG-1283 FeatureRegistry.featureMap concurrent access (issue669043)

[Paul Lindner <li...@inuus.com>]

My goal was to eliminate a bunch of arraylist creations on read-only methods by using immutable data structures internally.  As you can see it was a bit challenging.

Instead we might want to eliminate the optional injection of

   @Named("shindig.features.default") String featureFiles

Move it to the constructor, make register() private (or protected?) and be done with it, then no worries about concurrent access.  This is what I'd prefer, let Singletons be Singletons.

Is anyone subclassing this, would this be a big problem?


On Mar 23, 2010, at 4:47 PM, henry.saputra@gmail.com wrote:

> Actually I thought that the approach that Paul has suggested is to allow
> the read for the featureMap to make sure when new map is created, other
> threads could get the latest instance of it (the proposed change to
> replace creating new list to returning featureMap key set instead).
> 
> 
> http://codereview.appspot.com/669043/show

Re: SHINDIG-1283 FeatureRegistry.featureMap concurrent access (issue669043)

[Henry Saputra <he...@gmail.com>]

Hi Paul,

+1 for removing the set injection method and make register() method to be
protected/(package)private .

- Henry

On Tue, Mar 23, 2010 at 5:29 PM, Ziv Horesh <zh...@gmail.com> wrote:

> Yes move it to constructor would simplify it.
> GGS does not subclass it.
>
>
> On Tue, Mar 23, 2010 at 5:09 PM, Paul Lindner <li...@inuus.com> wrote:
>
> > My goal was to eliminate a bunch of arraylist creations on read-only
> > methods by using immutable data structures internally.  As you can see it
> > was a bit challenging.
> >
> > Instead we might want to eliminate the optional injection of
> >
> >   @Named("shindig.features.default") String featureFiles
> >
> > Move it to the constructor, make register() private (or protected?) and
> be
> > done with it, then no worries about concurrent access.  This is what I'd
> > prefer, let Singletons be Singletons.
> >
> > Is anyone subclassing this, would this be a big problem?
> >
> >
> > On Mar 23, 2010, at 4:47 PM, henry.saputra@gmail.com wrote:
> >
> > > Actually I thought that the approach that Paul has suggested is to
> allow
> > > the read for the featureMap to make sure when new map is created, other
> > > threads could get the latest instance of it (the proposed change to
> > > replace creating new list to returning featureMap key set instead).
> > >
> > >
> > > http://codereview.appspot.com/669043/show
> >
> >
>

Re: SHINDIG-1283 FeatureRegistry.featureMap concurrent access (issue669043)

[Ziv Horesh <zh...@gmail.com>]

Yes move it to constructor would simplify it.
GGS does not subclass it.


On Tue, Mar 23, 2010 at 5:09 PM, Paul Lindner <li...@inuus.com> wrote:

> My goal was to eliminate a bunch of arraylist creations on read-only
> methods by using immutable data structures internally.  As you can see it
> was a bit challenging.
>
> Instead we might want to eliminate the optional injection of
>
>   @Named("shindig.features.default") String featureFiles
>
> Move it to the constructor, make register() private (or protected?) and be
> done with it, then no worries about concurrent access.  This is what I'd
> prefer, let Singletons be Singletons.
>
> Is anyone subclassing this, would this be a big problem?
>
>
> On Mar 23, 2010, at 4:47 PM, henry.saputra@gmail.com wrote:
>
> > Actually I thought that the approach that Paul has suggested is to allow
> > the read for the featureMap to make sure when new map is created, other
> > threads could get the latest instance of it (the proposed change to
> > replace creating new list to returning featureMap key set instead).
> >
> >
> > http://codereview.appspot.com/669043/show
>
>