httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jim Jagielski <...@jaguNET.com>
Subject Re: slotmem API notes
Date Thu, 14 May 2009 12:56:59 GMT

On May 14, 2009, at 3:36 AM, jean-frederic clere wrote:
>
>>  It would also clear up some things I find odd at the moment;
>> for instance, why is there locking in the do() method but not
>> the put() method?
>
> The do operates on all slots put/get only in one... May be a flag in  
> the create. Something like ALLOW_LAZY_LOCKING?

Yeah... when a do is done, we want to ensure that
none of the slots change since we are touching all slots.
In general, we assume that with get and put, only one thread is
touch any particular slot at one time.

>>  It would be great to pass server_rec* and apr_pool_t* arguments
>> to all the methods (except maybe num_slots() and slot_size()).
>> Some providers may need to report error messages, and to do that
>> they really need a server_rec*.  They may also need to allocate
>> data from a temporary pool.  The most obvious use case is that
>> r->pool would be passed to a method like get() or put().
>
> I am -1 for passing a server_rec* because I think we should be able  
> to use the provider (via a wrapper) in something else that httpd.

+1 on the -1. The provider should report and error and the caller
do whatever is required to report that error. Having the
provider log is weird. For a temp pool, we do have a pool
for the provider, we could create a subpool.

>
>
>>  I'd also love to see create() split into two methods, create()
>> and init().  The create() method is then responsible for allocating
>> the ap_slotmem_instance_t structure and filling in the arguments;
>> it's also assumed that it will do any argument checks that are
>> required.  To this end, the typical use case is to call it in
>> the *first* invocation of the post_config hook, and pass the
>> pconf memory pool.  Note that this implies that create() takes
>> two apr_pool_t arguments, one for allocating "permanent" structures
>> which will last the lifetime of the instance, and one for temporary
>> allocations.  One would normally pass pconf and ptemp from the
>> post_config hook.
>
> +1

>
>
>>  The init() method then does all the work of actually setting
>> up the provider's data storage.  The typical use case would
>> be to call this from the *second* or subsequent invocations of
>> the post_config hook.  This is how the mod_socache_shmcb provider
>> functions and like mod_sharedmem is allocates shared memory segments.

I'm still not clear on the advantages of splitting...

>>
>>  I'd suggest removing both the mem() and attach() methods.
>>  The mem() method assumes that all providers will return a
>> slab of memory, and that writes to this memory are equivalent to
>> a put().  That's not going to be possible unless all providers
>> deal with memory -- in which case, the only two possible providers
>> that I can see are the "plain" and shared memory providers we have,
>> and then the purpose of a whole API and provider framework is lost
>> on me.  The mod_slotmem_zk provider I wrote, for example, just has
>> to return APR_ENOTIMPL for mem() because there's really nothing else
>> it can do.  Similarly any provider which uses a DB file or similar
>> backing store isn't going to be able to make mem() available.
>> Better, I think, to drop the method since it's so deeply tied to
>> an in-memory provider model.
>
> Well if we want to use it for the scoreboard mem() prevents memcpy().

I also thought that removing _mem made sense...

>>  Last but not least on the method front, I'd suggest adding
>> set() and reset() methods.  The set() method would work roughly like
>> memset() -- that is, it would accept a byte value and write that
>> into every byte in the slot.
>>  The reset() method would do the same as set() with a zero
>> byte value (i.e., blank the memory) but would also "release"
>> the slot.  The proposed grab() and return() methods would be
>> unnecessary.  A provider which wanted to track which slots were
>> "in use" would simply set the in-use flag when put(), set(), or
>> get() was called, and reset the flag when reset() was called.
>
> we have:
> grab/return
> put/get
> you want:
> put/set/get/reset.
> I do see a big difference, except I prefer the first one.

Yeah... I don't see the advantage for set/reset when we have get/put
since the end-user can do a set/reset via what they 'put'

Also, reset is 'release' now... we just need to "null-out" the
data (if really required...)

Mime
View raw message