libudev: Allocate udev_device->envp lazily

Allocate udev_device->envp lazily

It's a pity to allocate 128 words when we don't need them. Measured 2% _user_ cpu time reduction on EeePC coldplug.

Quoted 5 lines · Alan Jenkins

Allocate udev_device->envp lazily

It's a pity to allocate 128 words when we don't need them. Measured 2% _user_ cpu time reduction on EeePC coldplug.

Any idea what might take 2% here? We don't do anything if the envp isn't used. We do almost the same thing with or without the patch besides that "struct udev_device" is 1016 bytes smaller now, and we malloc() it.

Kay

Quoted 11 lines · Kay Sievers

Quoted 4 lines · Alan Jenkins

Allocate udev_device->envp lazily

It's a pity to allocate 128 words when we don't need them. Measured 2% _user_ cpu time reduction on EeePC coldplug.

Any idea what might take 2% here? We don't do anything if the envp isn't used. We do almost the same thing with or without the patch besides that "struct udev_device" is 1016 bytes smaller now, and we malloc() it.

oprofile said malloc() went down by about that much. I measured on top of threaded udevd, so I might be unlucky and chasing overheads you don't see in the single-threaded implementation :).

By rights it should reduce memset() (or calloc now). And it might help with caching, because the array effectively splits udev_device in two.

Allocate udev_device->envp lazily

It's a pity to allocate 128 words when we don't need them. Measured 2% _user_ cpu time reduction on EeePC coldplug.

Signed-off-by Alan Jenkins <[email protected]>

udev/lib/libudev-device.c +8 -3

index b54c727..69ab1f7 100644
@@ -31,6 +31,8 @@
 #include "libudev.h"
 #include "libudev-private.h"
 
+#define ENVP_SIZE 128
+
 struct udev_device {
       int refcount;
       struct udev *udev;

@@ -46,7 +48,7 @@ struct udev_device {
       struct udev_list_node devlinks_list;
       int devlinks_uptodate;
       struct udev_list_node properties_list;
-       char *envp[128];
+       char **envp;
       int envp_uptodate;
       char *driver;
       int driver_set;

@@ -611,8 +613,11 @@ void udev_device_unref(struct udev_device *udev_device)
       free(udev_device->devpath_old);
       free(udev_device->physdevpath);
       udev_list_cleanup_entries(udev_device->udev, &udev_device->sysattr_list);
-       for (i = 0; i < ARRAY_SIZE(udev_device->envp) && udev_device->envp[i] != NULL; i++)
-               free(udev_device->envp[i]);
+       if (udev_device->envp != NULL) {
+               for (i = 0; i < ENVP_SIZE && udev_device->envp[i] != NULL; i++)
+                       free(udev_device->envp[i]);
+               free(udev_device->envp);
+       }
       info(udev_device->udev, "udev_device: %p released\n", udev_device);
       free(udev_device);
 }

udev/lib/libudev-device.c +4 -0

+                               free(udev_device->envp[i]);
+               } else
+                       udev_device->envp = malloc(sizeof(char *) * ENVP_SIZE);
+

one small question ...

Quoted 47 lines · Alan Jenkins

Allocate udev_device->envp lazily

It's a pity to allocate 128 words when we don't need them. Measured 2% _user_ cpu time reduction on EeePC coldplug.

Signed-off-by Alan Jenkins <[email protected]>

udev/lib/libudev-device.c +8 -3

index b54c727..69ab1f7 100644
@@ -31,6 +31,8 @@
 #include "libudev.h"
 #include "libudev-private.h"
 
+#define ENVP_SIZE 128
+
 struct udev_device {
       int refcount;
       struct udev *udev;

@@ -46,7 +48,7 @@ struct udev_device {
       struct udev_list_node devlinks_list;
       int devlinks_uptodate;
       struct udev_list_node properties_list;
-       char *envp[128];
+       char **envp;
       int envp_uptodate;
       char *driver;
       int driver_set;

@@ -611,8 +613,11 @@ void udev_device_unref(struct udev_device *udev_device)
       free(udev_device->devpath_old);
       free(udev_device->physdevpath);
       udev_list_cleanup_entries(udev_device->udev, &udev_device->sysattr_list);
-       for (i = 0; i < ARRAY_SIZE(udev_device->envp) && udev_device->envp[i] != NULL; i++)
-               free(udev_device->envp[i]);
+       if (udev_device->envp != NULL) {
+               for (i = 0; i < ENVP_SIZE && udev_device->envp[i] != NULL; i++)
+                       free(udev_device->envp[i]);
+               free(udev_device->envp);
+       }
       info(udev_device->udev, "udev_device: %p released\n", udev_device);
       free(udev_device);
 }

@@ -1014,14 +1019,18 @@ char **udev_device_get_properties_envp(struct udev_device *udev_device)
               unsigned int i;
               struct udev_list_entry *list_entry;
 

udev/lib/libudev-device.c char **udev_device_get_properties_envp(struct udev_device *udev_device)

Quoted 5 lines

udev/lib/libudev-device.c +2 -2

-               for (i = 0; i < ARRAY_SIZE(udev_device->envp) && udev_device->envp[i] != NULL; i++)
-                       free(udev_device->envp[i]);
+               if (udev_device->envp) {
+                       for (i = 0; i < 128 && udev_device->envp[i] != NULL; i++)
 

should this 128 also be ENVP_SIZE?