diff mbox series

[RFC,4/5] libdrm: reduce number of reallocations in drmModeAtomicAddProperty

Message ID 1555734295-31015-5-git-send-email-john.stultz@linaro.org
State Superseded
Headers show
Series libdrm: Patches from AOSP | expand

Commit Message

John Stultz April 20, 2019, 4:24 a.m. UTC
From: Adrian Salido <salidoa@google.com>


When calling drmModeAtomicAddProperty allocation of memory happens as
needed in increments of 16 elements. This can be very slow if there are
multiple properties to be updated in an Atomic Commit call.

Increase this to as many as can fit in a memory PAGE to avoid having to
reallocate memory too often.

Cc: Emil Velikov <emil.velikov@collabora.com>
Cc: Sean Paul <seanpaul@chromium.org>
Cc: Alistair Strachan <astrachan@google.com>
Cc: Marissa Wall <marissaw@google.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>

---
 xf86drmMode.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

-- 
2.7.4

Comments

Alex Deucher April 22, 2019, 3:06 p.m. UTC | #1
On Sat, Apr 20, 2019 at 12:25 AM John Stultz <john.stultz@linaro.org> wrote:
>
> From: Adrian Salido <salidoa@google.com>
>
> When calling drmModeAtomicAddProperty allocation of memory happens as
> needed in increments of 16 elements. This can be very slow if there are
> multiple properties to be updated in an Atomic Commit call.
>
> Increase this to as many as can fit in a memory PAGE to avoid having to
> reallocate memory too often.
>
> Cc: Emil Velikov <emil.velikov@collabora.com>
> Cc: Sean Paul <seanpaul@chromium.org>
> Cc: Alistair Strachan <astrachan@google.com>
> Cc: Marissa Wall <marissaw@google.com>
> Signed-off-by: John Stultz <john.stultz@linaro.org>

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>  xf86drmMode.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/xf86drmMode.c b/xf86drmMode.c
> index 8f8633e..c878d9e 100644
> --- a/xf86drmMode.c
> +++ b/xf86drmMode.c
> @@ -1259,7 +1259,7 @@ drm_public drmModeAtomicReqPtr drmModeAtomicDuplicate(drmModeAtomicReqPtr old)
>                         return NULL;
>                 }
>                 memcpy(new->items, old->items,
> -                      old->size_items * sizeof(*new->items));
> +                      old->cursor * sizeof(*new->items));
>         } else {
>                 new->items = NULL;
>         }
> @@ -1322,12 +1322,13 @@ drm_public int drmModeAtomicAddProperty(drmModeAtomicReqPtr req,
>                 return -EINVAL;
>
>         if (req->cursor >= req->size_items) {
> +               const uint32_t item_size_inc = getpagesize() / sizeof(*req->items);
>                 drmModeAtomicReqItemPtr new;
>
> -               req->size_items += 16;
> +               req->size_items += item_size_inc;
>                 new = realloc(req->items, req->size_items * sizeof(*req->items));
>                 if (!new) {
> -                       req->size_items -= 16;
> +                       req->size_items -= item_size_inc;
>                         return -ENOMEM;
>                 }
>                 req->items = new;
> --
> 2.7.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Emil Velikov April 23, 2019, 10:12 a.m. UTC | #2
On Sat, 20 Apr 2019 at 05:25, John Stultz <john.stultz@linaro.org> wrote:
>
> From: Adrian Salido <salidoa@google.com>
>
> When calling drmModeAtomicAddProperty allocation of memory happens as
> needed in increments of 16 elements. This can be very slow if there are
> multiple properties to be updated in an Atomic Commit call.
>
> Increase this to as many as can fit in a memory PAGE to avoid having to
> reallocate memory too often.
>
> Cc: Emil Velikov <emil.velikov@collabora.com>
> Cc: Sean Paul <seanpaul@chromium.org>
> Cc: Alistair Strachan <astrachan@google.com>
> Cc: Marissa Wall <marissaw@google.com>
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> ---
>  xf86drmMode.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/xf86drmMode.c b/xf86drmMode.c
> index 8f8633e..c878d9e 100644
> --- a/xf86drmMode.c
> +++ b/xf86drmMode.c
> @@ -1259,7 +1259,7 @@ drm_public drmModeAtomicReqPtr drmModeAtomicDuplicate(drmModeAtomicReqPtr old)
>                         return NULL;
>                 }
>                 memcpy(new->items, old->items,
> -                      old->size_items * sizeof(*new->items));
> +                      old->cursor * sizeof(*new->items));
This seems like another (unrelated) perf. tweak. Splitting to separate
patch may be an overkill, so a trivial mention in the commit message
will do.

With that:
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>

-Emil
diff mbox series

Patch

diff --git a/xf86drmMode.c b/xf86drmMode.c
index 8f8633e..c878d9e 100644
--- a/xf86drmMode.c
+++ b/xf86drmMode.c
@@ -1259,7 +1259,7 @@  drm_public drmModeAtomicReqPtr drmModeAtomicDuplicate(drmModeAtomicReqPtr old)
 			return NULL;
 		}
 		memcpy(new->items, old->items,
-		       old->size_items * sizeof(*new->items));
+		       old->cursor * sizeof(*new->items));
 	} else {
 		new->items = NULL;
 	}
@@ -1322,12 +1322,13 @@  drm_public int drmModeAtomicAddProperty(drmModeAtomicReqPtr req,
 		return -EINVAL;
 
 	if (req->cursor >= req->size_items) {
+		const uint32_t item_size_inc = getpagesize() / sizeof(*req->items);
 		drmModeAtomicReqItemPtr new;
 
-		req->size_items += 16;
+		req->size_items += item_size_inc;
 		new = realloc(req->items, req->size_items * sizeof(*req->items));
 		if (!new) {
-			req->size_items -= 16;
+			req->size_items -= item_size_inc;
 			return -ENOMEM;
 		}
 		req->items = new;