Add in a series of micro-optimizations in rcutils

include/rcutils/logging.h

@@ -219,10 +219,6 @@ typedef void (* rcutils_logging_output_handler_t)(
   va_list *  // args
 );
 
-/// The function pointer of the current output handler.
-RCUTILS_PUBLIC
-extern rcutils_logging_output_handler_t g_rcutils_logging_output_handler;
-
 /// Get the current output handler.
 /**
  * <hr>
@@ -283,16 +279,6 @@ rcutils_ret_t rcutils_logging_format_message(
   int severity, const char * name, rcutils_time_point_value_t timestamp,
   const char * msg, rcutils_char_array_t * logging_output);
 
-/// The default severity level for loggers.
-/**
- * This level is used for (1) nameless log calls and (2) named log
- * calls where the effective level of the logger name is unspecified.
- *
- * \see rcutils_logging_get_logger_effective_level()
- */
-RCUTILS_PUBLIC
-extern int g_rcutils_logging_default_logger_level;
-
 /// Get the default level for loggers.
 /**
  * <hr>
@@ -345,7 +331,7 @@ void rcutils_logging_set_default_logger_level(int level);
  * \param[in] name The name of the logger, must be null terminated c string
  * \return The level of the logger if it has been set, or
  * \return `RCUTILS_LOG_SEVERITY_UNSET` if unset, or
- * \return `g_rcutils_logging_default_logger_level` for an empty name, or
+ * \return the default logger level for an empty name, or
  * \return -1 on invalid arguments, or
  * \return -1 if an error occurred
  */
@@ -370,7 +356,7 @@ int rcutils_logging_get_logger_level(const char * name);
  * \param[in] name_length Logger name length
  * \return The level of the logger if it has been set, or
  * \return `RCUTILS_LOG_SEVERITY_UNSET` if unset, or
- * \return `g_rcutils_logging_default_logger_level` for `name_length` of `0`, or
+ * \return the default logger level for an empty name, or
  * \return -1 on invalid arguments, or
  * \return -1 if an error occurred
  */
@@ -380,8 +366,7 @@ int rcutils_logging_get_logger_leveln(const char * name, size_t name_length);
 
 /// Set the severity level for a logger.
 /**
- * If an empty string is specified as the name, the
- * `g_rcutils_logging_default_logger_level` will be set.
+ * If an empty string is specified as the name, the default logger level will be set.
  *
  * <hr>
  * Attribute          | Adherence

include/rcutils/types/hash_map.h

@@ -156,7 +156,7 @@ rcutils_get_zero_initialized_hash_map();
  * ```
  *
  * \param[inout] hash_map rcutils_hash_map_t to be initialized
- * \param[in] initial_capacity the amount of initial capacity for the hash_map
+ * \param[in] initial_capacity the amount of initial capacity for the hash_map - this must be greater than zero and a power of 2
  * \param[in] key_size the size (in bytes) of the key used to index the data
  * \param[in] data_size the size (in bytes) of the data being stored
  * \param[in] key_hashing_func a function that returns a hashed value for a key

src/char_array.c

@@ -139,6 +139,12 @@ rcutils_char_array_expand_as_needed(rcutils_char_array_t * char_array, size_t ne
     return RCUTILS_RET_OK;
   }
 
+  // Make sure we expand by at least 1.5x the old capacity
+  size_t minimum_size = char_array->buffer_capacity + (char_array->buffer_capacity >> 1);
+  if (new_size < minimum_size) {
+    new_size = minimum_size;
+  }
+
   return rcutils_char_array_resize(char_array, new_size);
 }
 

src/find.c

@@ -12,11 +12,6 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#ifdef __cplusplus
-extern "C"
-{
-#endif
-
 #include <stdint.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -65,14 +60,19 @@ rcutils_find_lastn(const char * str, char delimiter, size_t string_length)
     return SIZE_MAX;
   }
 
+#if defined(_GNU_SOURCE)
+  const char * ptr = memrchr(str, delimiter, string_length);
+  if (ptr == NULL) {
+    return SIZE_MAX;
+  }
+
+  return ptr - str;
+#else
   for (size_t i = string_length - 1; i > 0; --i) {
     if (str[i] == delimiter) {
       return i;
     }
   }
   return str[0] == delimiter ? 0 : SIZE_MAX;
-}
-
-#ifdef __cplusplus
-}
 #endif
+}

src/hash_map.c

@@ -255,6 +255,13 @@ rcutils_hash_map_init(
     return RCUTILS_RET_INVALID_ARGUMENT;
   }
 
+  // Due to an optimization we use during lookup, we can currently only handle power-of-two
+  // capacities.  Enforce that here.
+  if ((initial_capacity & (initial_capacity - 1)) != 0) {
+    RCUTILS_SET_ERROR_MSG("This hashmap only works with power-of-two capacities");
+    return RCUTILS_RET_INVALID_ARGUMENT;
+  }
+
   hash_map->impl = allocator->allocate(sizeof(rcutils_hash_map_impl_t), allocator->state);
   if (NULL == hash_map->impl) {
     RCUTILS_SET_ERROR_MSG("failed to allocate memory for hash map impl");
@@ -329,7 +336,14 @@ static bool hash_map_find(
   rcutils_hash_map_entry_t * bucket_entry = NULL;
 
   *key_hash = hash_map->impl->key_hashing_func(key);
-  *map_index = (*key_hash) % hash_map->impl->capacity;
+  // The below is equivalent to:
+  //
+  // *map_index = (*key_hash) % hash_map->impl->capacity;
+  //
+  // This implementation is significantly faster since it avoids a divide, but
+  // only works when the capacity is a power of two.  We enforce that in the
+  // rcutils_hash_map_init() function.
+  *map_index = (*key_hash) & (hash_map->impl->capacity - 1);
 
   // Find the bucket the entry should be in check that it's valid
   rcutils_array_list_t * bucket = &(hash_map->impl->map[*map_index]);
@@ -394,7 +408,8 @@ rcutils_hash_map_set(rcutils_hash_map_t * hash_map, const void * key, const void
       memcpy(entry->value, value, hash_map->impl->data_size);
       memcpy(entry->key, key, hash_map->impl->key_size);
 
-      bucket_index = key_hash % hash_map->impl->capacity;
+      // See the comment in hash_map_find for why we do this.
+      bucket_index = key_hash & (hash_map->impl->capacity - 1);
       ret = hash_map_insert_entry(hash_map->impl->map, bucket_index, entry, allocator);
     }
 
@@ -424,6 +439,11 @@ rcutils_hash_map_unset(rcutils_hash_map_t * hash_map, const void * key)
   bool already_exists = false;
   rcutils_hash_map_entry_t * entry = NULL;
 
+  // If there is nothing in the hash map, don't bother computing the key
+  if (hash_map->impl->size == 0) {
+    return RCUTILS_RET_OK;
+  }
+
   already_exists = hash_map_find(hash_map, key, &key_hash, &map_index, &bucket_index, &entry);
 
   if (!already_exists) {
@@ -457,6 +477,11 @@ rcutils_hash_map_key_exists(const rcutils_hash_map_t * hash_map, const void * ke
   bool already_exists = false;
   rcutils_hash_map_entry_t * entry = NULL;
 
+  // If there is nothing in the hash map, don't bother computing the key
+  if (hash_map->impl->size == 0) {
+    return RCUTILS_RET_OK;
+  }
+
   already_exists = hash_map_find(hash_map, key, &key_hash, &map_index, &bucket_index, &entry);
 
   return already_exists;
@@ -473,6 +498,11 @@ rcutils_hash_map_get(const rcutils_hash_map_t * hash_map, const void * key, void
   bool already_exists = false;
   rcutils_hash_map_entry_t * entry = NULL;
 
+  // If there is nothing in the hash map, don't bother computing the key
+  if (hash_map->impl->size == 0) {
+    return RCUTILS_RET_NOT_FOUND;
+  }
+
   already_exists = hash_map_find(hash_map, key, &key_hash, &map_index, &bucket_index, &entry);
 
   if (already_exists) {
@@ -499,6 +529,15 @@ rcutils_hash_map_get_next_key_and_data(
   rcutils_hash_map_entry_t * entry = NULL;
   rcutils_ret_t ret = RCUTILS_RET_OK;
 
+  // If there is nothing in the hash map, don't bother computing the key
+  if (hash_map->impl->size == 0) {
+    if (NULL != previous_key) {
+      return RCUTILS_RET_NOT_FOUND;
+    } else {
+      return RCUTILS_RET_HASH_MAP_NO_MORE_ENTRIES;
+    }
+  }
+
   if (NULL != previous_key) {
     already_exists = hash_map_find(hash_map, key, &key_hash, &map_index, &bucket_index, &entry);
     if (!already_exists) {

src/logging.c

@@ -89,14 +89,14 @@ static const char * g_rcutils_logging_default_output_format =
 
 static rcutils_allocator_t g_rcutils_logging_allocator;
 
-rcutils_logging_output_handler_t g_rcutils_logging_output_handler = NULL;
+static rcutils_logging_output_handler_t g_rcutils_logging_output_handler = NULL;
 static rcutils_string_map_t g_rcutils_logging_severities_map;
 
 // If this is false, attempts to use the severities map will be skipped.
 // This can happen if allocation of the map fails at initialization.
 static bool g_rcutils_logging_severities_map_valid = false;
 
-int g_rcutils_logging_default_logger_level = 0;
+static int g_rcutils_logging_default_logger_level = 0;
 
 static FILE * g_output_stream = NULL;
 

test/benchmark/benchmark_logging.cpp

@@ -49,7 +49,7 @@ static void benchmark_logging(benchmark::State & state)
       ret_value = rcutils_logging_shutdown();
       (void) ret_value;
     });
-    g_rcutils_logging_default_logger_level = RCUTILS_LOG_SEVERITY_DEBUG;
+    rcutils_logging_set_default_logger_level(RCUTILS_LOG_SEVERITY_DEBUG);
 
     auto rcutils_logging_console_output_handler = [](
       const rcutils_log_location_t * location,

test/test_hash_map.cpp

@@ -95,6 +95,13 @@ TEST_F(HashMapBaseTest, init_map_initial_capacity_zero_fails) {
   EXPECT_EQ(RCUTILS_RET_INVALID_ARGUMENT, ret) << rcutils_get_error_string().str;
 }
 
+TEST_F(HashMapBaseTest, init_map_initial_capacity_not_power_of_two_fails) {
+  rcutils_ret_t ret = rcutils_hash_map_init(
+    &map, 3, sizeof(uint32_t), sizeof(uint32_t),
+    test_hash_map_uint32_hash_func, test_uint32_cmp, &allocator);
+  EXPECT_EQ(RCUTILS_RET_INVALID_ARGUMENT, ret) << rcutils_get_error_string().str;
+}
+
 TEST_F(HashMapBaseTest, init_map_key_size_zero_fails) {
   rcutils_ret_t ret = rcutils_hash_map_init(
     &map, 2, 0, sizeof(uint32_t),
@@ -480,7 +487,7 @@ TEST_F(HashMapBaseTest, string_keys) {
   const char * key2 = "two";
   const char * lookup_key = "one";
   rcutils_ret_t ret = rcutils_hash_map_init(
-    &map, 10, sizeof(char *), sizeof(uint32_t),
+    &map, 8, sizeof(char *), sizeof(uint32_t),
     rcutils_hash_map_string_hash_func, rcutils_hash_map_string_cmp_func,
     &allocator);
   EXPECT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str;

test/test_logging.cpp

@@ -67,8 +67,8 @@ TEST(TestLogging, test_logging) {
     EXPECT_EQ(RCUTILS_RET_OK, rcutils_logging_shutdown());
   });
   EXPECT_TRUE(g_rcutils_logging_initialized);
-  g_rcutils_logging_default_logger_level = RCUTILS_LOG_SEVERITY_DEBUG;
-  EXPECT_EQ(RCUTILS_LOG_SEVERITY_DEBUG, g_rcutils_logging_default_logger_level);
+  rcutils_logging_set_default_logger_level(RCUTILS_LOG_SEVERITY_DEBUG);
+  EXPECT_EQ(RCUTILS_LOG_SEVERITY_DEBUG, rcutils_logging_get_default_logger_level());
 
   auto rcutils_logging_console_output_handler = [](
     const rcutils_log_location_t * location,

test/test_logging_custom_env.cpp

@@ -30,7 +30,7 @@ TEST(TestLoggingCustomEnv, test_logging) {
     EXPECT_EQ(RCUTILS_RET_OK, rcutils_logging_shutdown());
   });
   EXPECT_TRUE(g_rcutils_logging_initialized);
-  g_rcutils_logging_default_logger_level = RCUTILS_LOG_SEVERITY_DEBUG;
+  rcutils_logging_set_default_logger_level(RCUTILS_LOG_SEVERITY_DEBUG);
 
   rcutils_ret_t ret;
   rcutils_allocator_t allocator = rcutils_get_default_allocator();

test/test_logging_macros.cpp

@@ -46,8 +46,8 @@ class TestLoggingMacros : public ::testing::Test
     EXPECT_FALSE(g_rcutils_logging_initialized);
     ASSERT_EQ(RCUTILS_RET_OK, rcutils_logging_initialize());
     EXPECT_TRUE(g_rcutils_logging_initialized);
-    g_rcutils_logging_default_logger_level = RCUTILS_LOG_SEVERITY_DEBUG;
-    EXPECT_EQ(RCUTILS_LOG_SEVERITY_DEBUG, g_rcutils_logging_default_logger_level);
+    rcutils_logging_set_default_logger_level(RCUTILS_LOG_SEVERITY_DEBUG);
+    EXPECT_EQ(RCUTILS_LOG_SEVERITY_DEBUG, rcutils_logging_get_default_logger_level());
 
     auto rcutils_logging_console_output_handler = [](
       const rcutils_log_location_t * location,
@@ -120,14 +120,14 @@ TEST_F(TestLoggingMacros, test_logging_function) {
   g_function_called = false;
 
   // check that evaluation of a specified function does not occur if the severity is not enabled
-  g_rcutils_logging_default_logger_level = RCUTILS_LOG_SEVERITY_INFO;
+  rcutils_logging_set_default_logger_level(RCUTILS_LOG_SEVERITY_INFO);
   for (int i : {0, 1}) {  // cover both true and false return values
     g_counter = i;
     RCUTILS_LOG_DEBUG_FUNCTION(&not_divisible_by_three, "message %d", i);
   }
   EXPECT_EQ(0u, g_log_calls);
   EXPECT_FALSE(g_function_called);
-  g_rcutils_logging_default_logger_level = RCUTILS_LOG_SEVERITY_DEBUG;
+  rcutils_logging_set_default_logger_level(RCUTILS_LOG_SEVERITY_DEBUG);
 
   for (int i : {1, 2, 3, 4, 5, 6}) {
     g_counter = i;