[PATCH v2 0/4] Optimize red-black tree insert/extract

[PATCH v2 0/4] Optimize red-black tree insert/extract

[Sebastian Huber]

Code coverage analysis of the red-black tree insert/extract operations defined
in <sys/tree.h> showed that the macros contain dead code.  This patch set
simplifies some expressions and add specialized rotations.

v2:

Add comments in patch 3 and 4.

Sebastian Huber (4):
  sys/tree.h: Simplify loop condition
  sys/tree.h: Simplify chain of conditions
  sys/tree.h: Add parent rotations
  sys/tree.h: Red child with black sibling rotations

 newlib/libc/include/sys/tree.h | 90 +++++++++++++++++++++++++++++++---
 1 file changed, 82 insertions(+), 8 deletions(-)

-- 
2.26.2

[PATCH v2] Align *utime*() with POSIX/glibc

[Sebastian Huber]

Change the prototypes to be in line with POSIX/glibc.  This may fix
issues with new warnings produced by GCC 11.

Signed-off-by: Sebastian Huber <sebastian.huber@embedded-brains.de>
---
 newlib/libc/include/sys/_default_fcntl.h | 2 +-
 newlib/libc/include/sys/stat.h           | 4 ++--
 newlib/libc/include/sys/time.h           | 6 +++---
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/newlib/libc/include/sys/_default_fcntl.h b/newlib/libc/include/sys/_default_fcntl.h
index b3177dd69..50a0de44b 100644
--- a/newlib/libc/include/sys/_default_fcntl.h
+++ b/newlib/libc/include/sys/_default_fcntl.h
@@ -221,7 +221,7 @@ extern int flock (int, int);
 #endif
 #if __GNU_VISIBLE
 #include <sys/time.h>
-extern int futimesat (int, const char *, const struct timeval *);
+extern int futimesat (int, const char *, const struct timeval [2]);
 #endif
 
 /* Provide _<systemcall> prototypes for functions provided by some versions
diff --git a/newlib/libc/include/sys/stat.h b/newlib/libc/include/sys/stat.h
index 8769112b0..722ed0eff 100644
--- a/newlib/libc/include/sys/stat.h
+++ b/newlib/libc/include/sys/stat.h
@@ -153,10 +153,10 @@ int	fstatat (int, const char *__restrict , struct stat *__restrict, int);
 int	mkdirat (int, const char *, mode_t);
 int	mkfifoat (int, const char *, mode_t);
 int	mknodat (int, const char *, mode_t, dev_t);
-int	utimensat (int, const char *, const struct timespec *, int);
+int	utimensat (int, const char *, const struct timespec [2], int);
 #endif
 #if __POSIX_VISIBLE >= 200809 && !defined(__INSIDE_CYGWIN__)
-int	futimens (int, const struct timespec *);
+int	futimens (int, const struct timespec [2]);
 #endif
 
 /* Provide prototypes for most of the _<systemcall> names that are
diff --git a/newlib/libc/include/sys/time.h b/newlib/libc/include/sys/time.h
index 84a429bf2..3be6c1e41 100644
--- a/newlib/libc/include/sys/time.h
+++ b/newlib/libc/include/sys/time.h
@@ -414,12 +414,12 @@ struct itimerval {
 #include <time.h>
 
 __BEGIN_DECLS
-int utimes (const char *__path, const struct timeval *__tvp);
+int utimes (const char *, const struct timeval [2]);
 
 #if __BSD_VISIBLE
 int adjtime (const struct timeval *, struct timeval *);
-int futimes (int, const struct timeval *);
-int lutimes (const char *, const struct timeval *);
+int futimes (int, const struct timeval [2]);
+int lutimes (const char *, const struct timeval [2]);
 int settimeofday (const struct timeval *, const struct timezone *);
 #endif
 
-- 
2.26.2

Re: [PATCH v2] Align *utime*() with POSIX/glibc

[Sebastian Huber]

On 05/10/2021 17:33, Sebastian Huber wrote:
> Change the prototypes to be in line with POSIX/glibc.  This may fix
> issues with new warnings produced by GCC 11.
> 
> Signed-off-by: Sebastian Huber <sebastian.huber@embedded-brains.de>

Sorry, please ignore this patch. I accidentally sent with in this patch set.

-- 
embedded brains GmbH
Herr Sebastian HUBER
Dornierstr. 4
82178 Puchheim
Germany
email: sebastian.huber@embedded-brains.de
phone: +49-89-18 94 741 - 16
fax:   +49-89-18 94 741 - 08

Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/

[PATCH v2 1/4] sys/tree.h: Simplify loop condition

[Sebastian Huber]

We have

  #define RB_ISRED(elm, field) \
    ((elm) != NULL && RB_COLOR(elm, field) == RB_RED)

So, the RB_ISRED() contains an implicit check for NULL.  In
RB_GENERATE_REMOVE_COLOR() the "elm" pointer cannot be NULL in the while
condition.  Use RB_COLOR(elm) == RB_BLACK instead.
---
 newlib/libc/include/sys/tree.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/newlib/libc/include/sys/tree.h b/newlib/libc/include/sys/tree.h
index 2af77a499..15831c7dd 100644
--- a/newlib/libc/include/sys/tree.h
+++ b/newlib/libc/include/sys/tree.h
@@ -540,7 +540,7 @@ name##_RB_REMOVE_COLOR(struct name *head, struct type *parent)		\
 			elm = RB_ROOT(head);				\
 			break;						\
 		}							\
-	} while (!RB_ISRED(elm, field) && parent != NULL);		\
+	} while (RB_COLOR(elm, field) == RB_BLACK && parent != NULL);	\
 	RB_COLOR(elm, field) = RB_BLACK;				\
 }
 
-- 
2.26.2

[PATCH v2 2/4] sys/tree.h: Simplify chain of conditions

[Sebastian Huber]

In RB_GENERATE_REMOVE_COLOR() simplify a chain of conditions of the following
pattern

if (x) {
  ...
} else if (!x) {
  ...
}

to

if (x) {
  ...
} else {
  ...
}
---
 newlib/libc/include/sys/tree.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/newlib/libc/include/sys/tree.h b/newlib/libc/include/sys/tree.h
index 15831c7dd..180809e9b 100644
--- a/newlib/libc/include/sys/tree.h
+++ b/newlib/libc/include/sys/tree.h
@@ -528,7 +528,7 @@ name##_RB_REMOVE_COLOR(struct name *head, struct type *parent)		\
 				RB_ROTATE_LEFT(head, tmp, oright, field); \
 				RB_COLOR(oright, field) = RB_BLACK;	\
 				tmp = oright;				\
-			} else if (!RB_ISRED(RB_LEFT(tmp, field), field)) { \
+			} else {					\
 				RB_COLOR(tmp, field) = RB_RED;		\
 				elm = parent;				\
 				parent = RB_PARENT(elm, field);		\
-- 
2.26.2

[PATCH v2 3/4] sys/tree.h: Add parent rotations

[Sebastian Huber]

Add specialized rotations RB_PARENT_ROTATE_LEFT() and RB_PARENT_ROTATE_RIGHT()
which may be used if the parent node exists and the direction of the child is
known.  The specialized rotations are derived from RB_ROTATE_LEFT() and
RB_ROTATE_RIGHT() where the RB_SWAP_CHILD() was replaced by a simple
assignment.
---
 newlib/libc/include/sys/tree.h | 43 ++++++++++++++++++++++++++++++----
 1 file changed, 39 insertions(+), 4 deletions(-)

diff --git a/newlib/libc/include/sys/tree.h b/newlib/libc/include/sys/tree.h
index 180809e9b..f6190f7f4 100644
--- a/newlib/libc/include/sys/tree.h
+++ b/newlib/libc/include/sys/tree.h
@@ -381,6 +381,37 @@ struct {								\
 	RB_AUGMENT(elm);						\
 } while (/*CONSTCOND*/ 0)
 
+/*
+ * The RB_PARENT_ROTATE_LEFT() and RB_PARENT_ROTATE_RIGHT() rotations are
+ * specialized versions of RB_ROTATE_LEFT() and RB_ROTATE_RIGHT() which may be
+ * used if the parent node exists and the direction of the child element is
+ * known.
+ */
+
+#define RB_PARENT_ROTATE_LEFT(parent, left, tmp, field) do {		\
+	(tmp) = RB_RIGHT(left, field);					\
+	if ((RB_RIGHT(left, field) = RB_LEFT(tmp, field)) != NULL) {	\
+		RB_SET_PARENT(RB_RIGHT(left, field), left, field);	\
+	}								\
+	RB_SET_PARENT(tmp, parent, field);				\
+	RB_LEFT(parent, field) = (tmp);					\
+	RB_LEFT(tmp, field) = (left);					\
+	RB_SET_PARENT(left, tmp, field);				\
+	RB_AUGMENT(left);						\
+} while (/*CONSTCOND*/ 0)
+
+#define RB_PARENT_ROTATE_RIGHT(parent, right, tmp, field) do {		\
+	(tmp) = RB_LEFT(right, field);					\
+	if ((RB_LEFT(right, field) = RB_RIGHT(tmp, field)) != NULL) {	\
+		RB_SET_PARENT(RB_LEFT(right, field), right, field);	\
+	}								\
+	RB_SET_PARENT(tmp, parent, field);				\
+	RB_RIGHT(parent, field) = (tmp);				\
+	RB_RIGHT(tmp, field) = (right);					\
+	RB_SET_PARENT(right, tmp, field);				\
+	RB_AUGMENT(right);						\
+} while (/*CONSTCOND*/ 0)
+
 /* Generates prototypes and inline functions */
 #define	RB_PROTOTYPE(name, type, field, cmp)				\
 	RB_PROTOTYPE_INTERNAL(name, type, field, cmp,)
@@ -454,7 +485,8 @@ name##_RB_INSERT_COLOR(struct name *head, struct type *elm)		\
 				continue;				\
 			}						\
 			if (RB_RIGHT(parent, field) == elm) {		\
-				RB_ROTATE_LEFT(head, parent, tmp, field);\
+				RB_PARENT_ROTATE_LEFT(gparent, parent,	\
+				    tmp, field);			\
 				tmp = parent;				\
 				parent = elm;				\
 				elm = tmp;				\
@@ -470,7 +502,8 @@ name##_RB_INSERT_COLOR(struct name *head, struct type *elm)		\
 				continue;				\
 			}						\
 			if (RB_LEFT(parent, field) == elm) {		\
-				RB_ROTATE_RIGHT(head, parent, tmp, field);\
+				RB_PARENT_ROTATE_RIGHT(gparent, parent,	\
+				    tmp, field);			\
 				tmp = parent;				\
 				parent = elm;				\
 				elm = tmp;				\
@@ -500,7 +533,8 @@ name##_RB_REMOVE_COLOR(struct name *head, struct type *parent)		\
 				RB_COLOR(RB_RIGHT(tmp, field), field) = RB_BLACK; \
 			else if (RB_ISRED(RB_LEFT(tmp, field), field)) { \
 				struct type *oleft;			\
-				RB_ROTATE_RIGHT(head, tmp, oleft, field); \
+				RB_PARENT_ROTATE_RIGHT(parent, tmp,	\
+				    oleft, field);			\
 				RB_COLOR(oleft, field) = RB_BLACK;	\
 				tmp = oleft;				\
 			} else {					\
@@ -525,7 +559,8 @@ name##_RB_REMOVE_COLOR(struct name *head, struct type *parent)		\
 				RB_COLOR(RB_LEFT(tmp, field), field) = RB_BLACK; \
 			else if (RB_ISRED(RB_RIGHT(tmp, field), field)) { \
 				struct type *oright;			\
-				RB_ROTATE_LEFT(head, tmp, oright, field); \
+				RB_PARENT_ROTATE_LEFT(parent, tmp,	\
+				    oright, field);			\
 				RB_COLOR(oright, field) = RB_BLACK;	\
 				tmp = oright;				\
 			} else {					\
-- 
2.26.2

[PATCH v2 4/4] sys/tree.h: Red child with black sibling rotations

[Sebastian Huber]

Add specialized rotations RB_RED_ROTATE_LEFT() and RB_RED_ROTATE_RIGHT() which
may be used if we rotate a red child which has a black sibling.  Such a red
node must have at least two child nodes so that the following red-black tree
invariant is fulfilled:

  Every path from a given node to any of its descendant NULL nodes goes through
  the same number of black nodes.

      PARENT
     /      \
  BLACK     RED
           /   \
        BLACK BLACK
---
 newlib/libc/include/sys/tree.h | 43 ++++++++++++++++++++++++++++++++--
 1 file changed, 41 insertions(+), 2 deletions(-)

diff --git a/newlib/libc/include/sys/tree.h b/newlib/libc/include/sys/tree.h
index f6190f7f4..7a3b55bdf 100644
--- a/newlib/libc/include/sys/tree.h
+++ b/newlib/libc/include/sys/tree.h
@@ -412,6 +412,45 @@ struct {								\
 	RB_AUGMENT(right);						\
 } while (/*CONSTCOND*/ 0)
 
+/*
+ * The RB_RED_ROTATE_LEFT() and RB_RED_ROTATE_RIGHT() rotations are specialized
+ * versions of RB_ROTATE_LEFT() and RB_ROTATE_RIGHT() which may be used if we
+ * rotate a red child element which has a black sibling.  Such a red node must
+ * have at least two child nodes so that the following red-black tree invariant
+ * is fulfilled:
+ *
+ *  Every path from a given node to any of its descendant NULL nodes goes
+ *  through the same number of black nodes.
+ *
+ *     parent
+ *    /      \
+ * BLACK   RED (elm)
+ *          /   \
+ *       BLACK  BLACK
+ */
+
+#define RB_RED_ROTATE_LEFT(head, elm, tmp, field) do {			\
+	(tmp) = RB_RIGHT(elm, field);					\
+	RB_RIGHT(elm, field) = RB_LEFT(tmp, field);			\
+	RB_SET_PARENT(RB_RIGHT(elm, field), elm, field);		\
+	RB_SET_PARENT(tmp, RB_PARENT(elm, field), field);		\
+	RB_SWAP_CHILD(head, elm, tmp, field);				\
+	RB_LEFT(tmp, field) = (elm);					\
+	RB_SET_PARENT(elm, tmp, field);					\
+	RB_AUGMENT(elm);						\
+} while (/*CONSTCOND*/ 0)
+
+#define RB_RED_ROTATE_RIGHT(head, elm, tmp, field) do {			\
+	(tmp) = RB_LEFT(elm, field);					\
+	RB_LEFT(elm, field) = RB_RIGHT(tmp, field);			\
+	RB_SET_PARENT(RB_LEFT(elm, field), elm, field);			\
+	RB_SET_PARENT(tmp, RB_PARENT(elm, field), field);		\
+	RB_SWAP_CHILD(head, elm, tmp, field);				\
+	RB_RIGHT(tmp, field) = (elm);					\
+	RB_SET_PARENT(elm, tmp, field);					\
+	RB_AUGMENT(elm);						\
+} while (/*CONSTCOND*/ 0)
+
 /* Generates prototypes and inline functions */
 #define	RB_PROTOTYPE(name, type, field, cmp)				\
 	RB_PROTOTYPE_INTERNAL(name, type, field, cmp,)
@@ -526,7 +565,7 @@ name##_RB_REMOVE_COLOR(struct name *head, struct type *parent)		\
 			tmp = RB_RIGHT(parent, field);			\
 			if (RB_COLOR(tmp, field) == RB_RED) {		\
 				RB_SET_BLACKRED(tmp, parent, field);	\
-				RB_ROTATE_LEFT(head, parent, tmp, field);\
+				RB_RED_ROTATE_LEFT(head, parent, tmp, field); \
 				tmp = RB_RIGHT(parent, field);		\
 			}						\
 			if (RB_ISRED(RB_RIGHT(tmp, field), field))	\
@@ -552,7 +591,7 @@ name##_RB_REMOVE_COLOR(struct name *head, struct type *parent)		\
 			tmp = RB_LEFT(parent, field);			\
 			if (RB_COLOR(tmp, field) == RB_RED) {		\
 				RB_SET_BLACKRED(tmp, parent, field);	\
-				RB_ROTATE_RIGHT(head, parent, tmp, field);\
+				RB_RED_ROTATE_RIGHT(head, parent, tmp, field); \
 				tmp = RB_LEFT(parent, field);		\
 			}						\
 			if (RB_ISRED(RB_LEFT(tmp, field), field))	\
-- 
2.26.2

Re: [PATCH v2 0/4] Optimize red-black tree insert/extract

[Jeff Johnston]

Hi Sebastian,

LGTM.  If no one else objects, I will merge tomorrow.

-- Jeff J.

On Tue, Oct 5, 2021 at 11:34 AM Sebastian Huber <
sebastian.huber@embedded-brains.de> wrote:

> Code coverage analysis of the red-black tree insert/extract operations
> defined
> in <sys/tree.h> showed that the macros contain dead code.  This patch set
> simplifies some expressions and add specialized rotations.
>
> v2:
>
> Add comments in patch 3 and 4.
>
> Sebastian Huber (4):
>   sys/tree.h: Simplify loop condition
>   sys/tree.h: Simplify chain of conditions
>   sys/tree.h: Add parent rotations
>   sys/tree.h: Red child with black sibling rotations
>
>  newlib/libc/include/sys/tree.h | 90 +++++++++++++++++++++++++++++++---
>  1 file changed, 82 insertions(+), 8 deletions(-)
>
> --
> 2.26.2
>
>

Re: [PATCH v2 0/4] Optimize red-black tree insert/extract

[Sebastian Huber]

Hello Jeff,

On 06/10/2021 20:10, Jeff Johnston wrote:
> Hi Sebastian,
> 
> LGTM.  If no one else objects, I will merge tomorrow.

thanks for having a look at it. I can check it in myself if this is 
easier for you.

-- 
embedded brains GmbH
Herr Sebastian HUBER
Dornierstr. 4
82178 Puchheim
Germany
email: sebastian.huber@embedded-brains.de
phone: +49-89-18 94 741 - 16
fax:   +49-89-18 94 741 - 08

Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/