Re: svn commit: r1893004 - /httpd/httpd/trunk/modules/metadata/mod_unique_id.c

[Yann Ylavic]

On Tue, Sep 7, 2021 at 8:07 AM Ruediger Pluem  wrote:
>
> On 9/7/21 3:52 AM, yla...@apache.org wrote:
> >
> > -/* Set counter in network byte order for the uuencoded unique id. */
> > -new_unique_id.counter = htons(new_unique_id.counter);
> > +/* The counter is two bytes for the uuencoded unique id, in network
> > + * byte order.
> > + */
> > +new_unique_id.counter = htons(counter % APR_UINT16_MAX);
>
> Isn't '&' doing the same as '%' here and is more performant?

With a *literal* ^2-1 modulus, the compiler generates the exact same
code (a '&'). The modulo is possibly more readable..
But yeah, if the modulus were a variable, the '&' is clearly more performant.


Regards;
Yann.

Re: svn commit: r1893004 - /httpd/httpd/trunk/modules/metadata/mod_unique_id.c

[Ruediger Pluem]

On 9/7/21 3:52 AM, yla...@apache.org wrote:
> Author: ylavic
> Date: Tue Sep  7 01:52:48 2021
> New Revision: 1893004
> 
> URL: http://svn.apache.org/viewvc?rev=1893004&view=rev
> Log:
> mod_unique_id: Follow up to r1892915 and r1893002: Atomic counter.
> 
> * modules/metadata/mod_unique_id.c(gen_unique_id):
>   Use an atomic 32bit counter to close the race condition, using the lower
>   16 bits for uuencoding still.
> 
> 
> Modified:
> httpd/httpd/trunk/modules/metadata/mod_unique_id.c
> 
> Modified: httpd/httpd/trunk/modules/metadata/mod_unique_id.c
> URL: 
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/metadata/mod_unique_id.c?rev=1893004&r1=1893003&r2=1893004&view=diff
> ==
> --- httpd/httpd/trunk/modules/metadata/mod_unique_id.c (original)
> +++ httpd/httpd/trunk/modules/metadata/mod_unique_id.c Tue Sep  7 01:52:48 
> 2021

> @@ -194,16 +212,24 @@ static const char *gen_unique_id(const r
>  unique_id_rec foo;
>  unsigned char pad[2];
>  } paddedbuf;
> +apr_uint32_t counter;
>  unsigned char *x,*y;
>  int i,j,k;
>  
>  memcpy(&new_unique_id.root, &cur_unique_id.root, ROOT_SIZE);
>  new_unique_id.stamp = htonl((unsigned int)apr_time_sec(r->request_time));
>  new_unique_id.thread_index = htonl((unsigned int)r->connection->id);
> -new_unique_id.counter = cur_unique_id.counter++;
> +#ifdef APR_HAS_THREADS
> +if (is_threaded_mpm)
> +counter = apr_atomic_inc32(&cur_unique_counter);
> +else
> +#endif
> +counter = cur_unique_counter++;
>  
> -/* Set counter in network byte order for the uuencoded unique id. */
> -new_unique_id.counter = htons(new_unique_id.counter);
> +/* The counter is two bytes for the uuencoded unique id, in network
> + * byte order.
> + */
> +new_unique_id.counter = htons(counter % APR_UINT16_MAX);

Isn't '&' doing the same as '%' here and is more performant?

Regards

Rüdiger