You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by yl...@apache.org on 2021/09/07 01:52:48 UTC
svn commit: r1893004 - /httpd/httpd/trunk/modules/metadata/mod_unique_id.c
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
@@ -26,6 +26,11 @@
#include "apr_general.h" /* for APR_OFFSETOF */
#include "apr_network_io.h"
+#ifdef APR_HAS_THREADS
+#include "apr_atomic.h" /* for apr_atomic_inc32 */
+#include "mpm_common.h" /* for ap_mpm_query */
+#endif
+
#include "httpd.h"
#include "http_config.h"
#include "http_log.h"
@@ -123,6 +128,10 @@ typedef struct {
* XXX: thrashing.
*/
static unique_id_rec cur_unique_id;
+static apr_uint32_t cur_unique_counter;
+#ifdef APR_HAS_THREADS
+static int is_threaded_mpm;
+#endif
/*
* Number of elements in the structure unique_id_rec.
@@ -160,6 +169,11 @@ static int unique_id_global_init(apr_poo
static void unique_id_child_init(apr_pool_t *p, server_rec *s)
{
+#ifdef APR_HAS_THREADS
+ is_threaded_mpm = 0;
+ ap_mpm_query(AP_MPMQ_IS_THREADED, &is_threaded_mpm);
+#endif
+
ap_random_insecure_bytes(&cur_unique_id.root,
sizeof(cur_unique_id.root));
@@ -168,8 +182,8 @@ static void unique_id_child_init(apr_poo
* against restart problems, and a little less protection against a clock
* going backwards in time.
*/
- ap_random_insecure_bytes(&cur_unique_id.counter,
- sizeof(cur_unique_id.counter));
+ ap_random_insecure_bytes(&cur_unique_counter,
+ sizeof(cur_unique_counter));
}
/* Use the base64url encoding per RFC 4648, avoiding characters which
@@ -182,6 +196,10 @@ static const char uuencoder[64] = {
'0', '1', '2', '3', '4', '5', '6', '7', '8', '9', '-', '_',
};
+#ifndef APR_UINT16_MAX
+#define APR_UINT16_MAX 0xffffu
+#endif
+
static const char *gen_unique_id(const request_rec *r)
{
char *str;
@@ -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);
/* we'll use a temporal buffer to avoid uuencoding the possible internal
* paddings of the original structure */
Re: svn commit: r1893004 - /httpd/httpd/trunk/modules/metadata/mod_unique_id.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Tue, Sep 7, 2021 at 8:07 AM Ruediger Pluem <rp...@apache.org> wrote:
>
> On 9/7/21 3:52 AM, ylavic@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
Posted by Ruediger Pluem <rp...@apache.org>.
On 9/7/21 3:52 AM, ylavic@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