On 2013-12-20 14:10:57 -0500, Robert Haas wrote:
> > Since you're embedding spinlocks in struct shm_toc, this module will be
> > in conflict with platforms that do --disable-spinlocks, since the number
> > of spinlocks essentially needs to be predetermined there. I personally
> > still think the solution to that is getting rid of --disable-spinlocks.
>
> Yep. We can either deprecate --disable-spinlocks, or we can add an
> API that is the reverse of S_INIT_LOCK().
How is that going to help? Even if we'd deallocate unused spinlocks -
which sure is a good idea when they require persistent resources like
the PGSemaphore based one - the maximum is still going to be there.
> > I vote for removing all the shm_toc_estimator() knowledge from the
> > header, there seems little reason to expose it that way. That just
> > exposes unneccessary details and makes fixes after releases harder
> > (requiring extensions to recompile). Function call costs hardly can
> > matter, right?
>
> Oh, you're saying to make it a function rather than a macro? Sure, I
> could do that.
Yea, keeping it private, as a function, seems like a good idea.
> > Do you assume that lookup speed isn't that important? I have a bit of a
> > hard time imagining that linear search over the keys isn't going to bite
> > us. At the least there should be a comment somewhere documenting that
> > the indented usecase is having a relatively few keys.
>
> Well, as previously noted, in the use cases I imagine for this, it'll
> be nworkers + somesmallconstant. I can add a comment to that effect.
I primarily was wondering because you have gone through a bit of effort
making it lockless. A comment would be good, yes.
Greetings,
Andres Freund
-- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services