From 26293609878093145d693a67cdd2ee0be139b9ac Mon Sep 17 00:00:00 2001 From: Jean-Roland Date: Wed, 18 Dec 2024 16:26:43 +0100 Subject: [PATCH 01/15] feat: add lru cache module and tests --- CMakeLists.txt | 3 + include/zenoh-pico/collections/lru_cache.h | 69 +++++ src/collections/lru_cache.c | 324 +++++++++++++++++++++ tests/z_lru_cache_test.c | 213 ++++++++++++++ 4 files changed, 609 insertions(+) create mode 100644 include/zenoh-pico/collections/lru_cache.h create mode 100644 src/collections/lru_cache.c create mode 100644 tests/z_lru_cache_test.c diff --git a/CMakeLists.txt b/CMakeLists.txt index 2a8233b12..f9fd2e845 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -503,6 +503,7 @@ if(UNIX OR MSVC) add_executable(z_api_bytes_test ${PROJECT_SOURCE_DIR}/tests/z_api_bytes_test.c) add_executable(z_api_encoding_test ${PROJECT_SOURCE_DIR}/tests/z_api_encoding_test.c) add_executable(z_refcount_test ${PROJECT_SOURCE_DIR}/tests/z_refcount_test.c) + add_executable(z_lru_cache_test ${PROJECT_SOURCE_DIR}/tests/z_lru_cache_test.c) target_link_libraries(z_data_struct_test zenohpico::lib) target_link_libraries(z_channels_test zenohpico::lib) @@ -521,6 +522,7 @@ if(UNIX OR MSVC) target_link_libraries(z_api_bytes_test zenohpico::lib) target_link_libraries(z_api_encoding_test zenohpico::lib) target_link_libraries(z_refcount_test zenohpico::lib) + target_link_libraries(z_lru_cache_test zenohpico::lib) configure_file(${PROJECT_SOURCE_DIR}/tests/modularity.py ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/modularity.py COPYONLY) configure_file(${PROJECT_SOURCE_DIR}/tests/raweth.py ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/raweth.py COPYONLY) @@ -545,6 +547,7 @@ if(UNIX OR MSVC) add_test(z_api_bytes_test ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/z_api_bytes_test) add_test(z_api_encoding_test ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/z_api_encoding_test) add_test(z_refcount_test ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/z_refcount_test) + add_test(z_lru_cache_test ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/z_lru_cache_test) endif() if(BUILD_MULTICAST) diff --git a/include/zenoh-pico/collections/lru_cache.h b/include/zenoh-pico/collections/lru_cache.h new file mode 100644 index 000000000..5c46271ea --- /dev/null +++ b/include/zenoh-pico/collections/lru_cache.h @@ -0,0 +1,69 @@ +// +// Copyright (c) 2024 ZettaScale Technology +// +// This program and the accompanying materials are made available under the +// terms of the Eclipse Public License 2.0 which is available at +// http://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 +// which is available at https://www.apache.org/licenses/LICENSE-2.0. +// +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// +// Contributors: +// ZettaScale Zenoh Team, +// + +#ifndef ZENOH_PICO_COLLECTIONS_LRUCACHE_H +#define ZENOH_PICO_COLLECTIONS_LRUCACHE_H + +#include + +#include "zenoh-pico/collections/element.h" + +#ifdef __cplusplus +extern "C" { +#endif + +// TODO: move to config +#define Z_FEATURE_CACHE_TREE 0 + +// Three way comparison function pointer +typedef int (*_z_lru_val_cmp_f)(const void *first, const void *second); + +// Node struct: {node_data; generic type} +typedef void _z_lru_cache_node_t; + +/*-------- Dynamically allocated vector --------*/ +/** + * A least recently used cache implementation + */ +typedef struct _z_lru_cache_t { + size_t capacity; // Max number of node + size_t len; // Number of node + _z_lru_cache_node_t *head; // List head + _z_lru_cache_node_t *tail; // List tail +#if Z_FEATURE_CACHE_TREE == 1 + _z_lru_cache_node_t *root; // Tree root +#endif +} _z_lru_cache_t; + +_z_lru_cache_t _z_lru_cache_init(size_t capacity); +void *_z_lru_cache_get(_z_lru_cache_t *cache, void *value, _z_lru_val_cmp_f compare); +z_result_t _z_lru_cache_insert(_z_lru_cache_t *cache, void *value, size_t value_size, _z_lru_val_cmp_f compare); +void _z_lru_cache_delete(_z_lru_cache_t *cache); + +#define _Z_LRU_CACHE_DEFINE(name, type, compare_f) \ + typedef _z_lru_cache_t name##_lru_cache_t; \ + static inline name##_lru_cache_t name##_lru_cache_init(size_t capacity) { return _z_lru_cache_init(capacity); } \ + static inline type *name##_lru_cache_get(name##_lru_cache_t *cache, type *val) { \ + return (type *)_z_lru_cache_get(cache, (void *)val, compare_f); \ + } \ + static inline z_result_t name##_lru_cache_insert(name##_lru_cache_t *cache, type *val) { \ + return _z_lru_cache_insert(cache, (void *)val, sizeof(type), compare_f); \ + } \ + static inline void name##_lru_cache_delete(name##_lru_cache_t *cache) { _z_lru_cache_delete(cache); } + +#ifdef __cplusplus +} +#endif + +#endif /* ZENOH_PICO_COLLECTIONS_LRUCACHE_H */ diff --git a/src/collections/lru_cache.c b/src/collections/lru_cache.c new file mode 100644 index 000000000..0bbcbf381 --- /dev/null +++ b/src/collections/lru_cache.c @@ -0,0 +1,324 @@ +// +// Copyright (c) 2024 ZettaScale Technology +// +// This program and the accompanying materials are made available under the +// terms of the Eclipse Public License 2.0 which is available at +// http://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 +// which is available at https://www.apache.org/licenses/LICENSE-2.0. +// +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// +// Contributors: +// ZettaScale Zenoh Team, +// + +#include "zenoh-pico/collections/lru_cache.h" + +#include +#include +#include + +#include "zenoh-pico/system/common/platform.h" +#include "zenoh-pico/utils/logging.h" +#include "zenoh-pico/utils/pointers.h" +#include "zenoh-pico/utils/result.h" + +// Nodes are chained both as a binary tree for lookup and as double linked list for lru insertion/deletion. +typedef struct _z_lru_cache_node_data_t { + _z_lru_cache_node_t *prev; // List previous node + _z_lru_cache_node_t *next; // List next node +#if Z_FEATURE_CACHE_TREE == 1 + _z_lru_cache_node_t *left; // Tree left child + _z_lru_cache_node_t *right; // Tree right child +#endif +} _z_lru_cache_node_data_t; + +#define NODE_DATA_SIZE sizeof(_z_lru_cache_node_data_t) + +// Generic static functions +static inline _z_lru_cache_t _z_lru_cache_null(void) { return (_z_lru_cache_t){0}; } + +static inline _z_lru_cache_node_data_t *_z_lru_cache_node_data(_z_lru_cache_node_t *node) { + return (_z_lru_cache_node_data_t *)node; +} + +static inline void *_z_lru_cache_node_value(_z_lru_cache_node_t *node) { + return (void *)_z_ptr_u8_offset((uint8_t *)node, (ptrdiff_t)NODE_DATA_SIZE); +} + +static _z_lru_cache_node_t *_z_lru_cache_node_create(void *value, size_t value_size) { + size_t node_size = NODE_DATA_SIZE + value_size; + _z_lru_cache_node_t *node = z_malloc(node_size); + if (node == NULL) { + return node; + } + memset(node, 0, NODE_DATA_SIZE); + memcpy(_z_lru_cache_node_value(node), value, value_size); + return node; +} + +// List functions +static void _z_lru_cache_insert_list_node(_z_lru_cache_t *cache, _z_lru_cache_node_t *node) { + _z_lru_cache_node_data_t *node_data = _z_lru_cache_node_data(node); + node_data->prev = NULL; + node_data->next = cache->head; + + if (cache->head != NULL) { + _z_lru_cache_node_data_t *head_data = _z_lru_cache_node_data(cache->head); + head_data->prev = node; + } + cache->head = node; + if (cache->tail == NULL) { + cache->tail = node; + } +} + +static void _z_lru_cache_remove_list_node(_z_lru_cache_t *cache, _z_lru_cache_node_t *node) { + _z_lru_cache_node_data_t *node_data = _z_lru_cache_node_data(node); + + // Nominal case + if ((node_data->prev != NULL) && (node_data->next != NULL)) { + _z_lru_cache_node_data_t *prev_data = _z_lru_cache_node_data(node_data->prev); + _z_lru_cache_node_data_t *next_data = _z_lru_cache_node_data(node_data->next); + prev_data->next = node_data->next; + next_data->prev = node_data->prev; + } + if (node_data->prev == NULL) { + assert(cache->head == node); + cache->head = node_data->next; + if (node_data->next != NULL) { + _z_lru_cache_node_data_t *next_data = _z_lru_cache_node_data(node_data->next); + next_data->prev = NULL; + } + } + if (node_data->next == NULL) { + assert(cache->tail == node); + cache->tail = node_data->prev; + if (node_data->prev != NULL) { + _z_lru_cache_node_data_t *prev_data = _z_lru_cache_node_data(node_data->prev); + prev_data->next = NULL; + } + } +} + +static void _z_lru_cache_update_list(_z_lru_cache_t *cache, _z_lru_cache_node_t *node) { + _z_lru_cache_remove_list_node(cache, node); + _z_lru_cache_insert_list_node(cache, node); +} + +// Tree functions +#if Z_FEATURE_CACHE_TREE == 1 +static _z_lru_cache_node_t *_z_lru_cache_insert_tree_node_rec(_z_lru_cache_node_t *previous, _z_lru_cache_node_t *node, + _z_lru_val_cmp_f compare) { + if (previous == NULL) { + return node; + } + _z_lru_cache_node_data_t *prev_data = _z_lru_cache_node_data(previous); + int res = compare(_z_lru_cache_node_value(previous), _z_lru_cache_node_value(node)); + if (res < 0) { + prev_data->left = _z_lru_cache_insert_tree_node_rec(prev_data->left, node, compare); + } else if (res > 0) { + prev_data->right = _z_lru_cache_insert_tree_node_rec(prev_data->right, node, compare); + } + // == 0 shouldn't happen + else { + _Z_ERROR("Comparison equality shouldn't happen during tree insertion"); + } + return previous; +} + +static void _z_lru_cache_insert_tree_node(_z_lru_cache_t *cache, _z_lru_cache_node_t *node, _z_lru_val_cmp_f compare) { + cache->root = _z_lru_cache_insert_tree_node_rec(cache->root, node, compare); +} + +static _z_lru_cache_node_t *_z_lru_cache_search_tree_rec(_z_lru_cache_node_t *node, void *value, + _z_lru_val_cmp_f compare) { + // Check value + if (node == NULL) { + return NULL; + } + // Compare keys + int res = compare(_z_lru_cache_node_value(node), value); + // Node has key + if (res == 0) { + return node; + } + _z_lru_cache_node_data_t *node_data = _z_lru_cache_node_data(node); + // Key is greater than node's key + if (res > 0) { + return _z_lru_cache_search_tree_rec(node_data->right, value, compare); + } + // Key is smaller than node's key + return _z_lru_cache_search_tree_rec(node_data->left, value, compare); +} + +static _z_lru_cache_node_t *_z_lru_cache_search_tree(_z_lru_cache_t *cache, void *value, _z_lru_val_cmp_f compare) { + return _z_lru_cache_search_tree_rec(cache->root, value, compare); +} + +static _z_lru_cache_node_t *_z_lru_cache_get_tree_node_successor(_z_lru_cache_node_t *node) { + _z_lru_cache_node_data_t *node_data = _z_lru_cache_node_data(node); + node = node_data->right; + node_data = _z_lru_cache_node_data(node); + while ((node != NULL) && (node_data->left != NULL)) { + node = node_data->left; + node_data = _z_lru_cache_node_data(node); + } + return node; +} + +static void _z_lru_cache_transfer_tree_node(_z_lru_cache_node_t *dst, _z_lru_cache_node_t *src, size_t value_size) { + void *src_val = _z_lru_cache_node_value(src); + void *dst_val = _z_lru_cache_node_value(dst); + _z_lru_cache_node_data_t *src_data = _z_lru_cache_node_data(src); + _z_lru_cache_node_data_t *dst_data = _z_lru_cache_node_data(dst); + // Copy content + memcpy(dst_val, src_val, value_size); + dst_data->prev = src_data->prev; + dst_data->next = src_data->next; + // Update list + if (src_data->prev != NULL) { + _z_lru_cache_node_data_t *prev_data = _z_lru_cache_node_data(src_data->prev); + prev_data->next = dst; + } + if (src_data->next != NULL) { + _z_lru_cache_node_data_t *next_data = _z_lru_cache_node_data(src_data->next); + next_data->prev = dst; + } +} + +static _z_lru_cache_node_t *_z_lru_cache_delete_tree_node_rec(_z_lru_cache_node_t *node, void *value, size_t value_size, + _z_lru_val_cmp_f compare) { + // No node + if (node == NULL) { + return NULL; + } + // Compare value + _z_lru_cache_node_data_t *node_data = _z_lru_cache_node_data(node); + void *node_val = _z_lru_cache_node_value(node); + int res = compare(node_val, value); + if (res < 0) { + node_data->left = _z_lru_cache_delete_tree_node_rec(node_data->left, value, value_size, compare); + } else if (res > 0) { + node_data->right = _z_lru_cache_delete_tree_node_rec(node_data->right, value, value_size, compare); + } else { // Node found + // No child / one child + if (node_data->left == NULL) { + _z_lru_cache_node_t *temp = node_data->right; + z_free(node); + return temp; + } else if (node_data->right == NULL) { + _z_lru_cache_node_t *temp = node_data->left; + z_free(node); + return temp; + } + // Two children + _z_lru_cache_node_t *succ = _z_lru_cache_get_tree_node_successor(node); + // Transfer successor data to node + _z_lru_cache_transfer_tree_node(node, succ, value_size); + node_data->right = + _z_lru_cache_delete_tree_node_rec(node_data->right, _z_lru_cache_node_value(succ), value_size, compare); + } + return node; +} + +static void _z_lru_cache_delete_tree_node(_z_lru_cache_t *cache, _z_lru_cache_node_t *node, size_t value_size, + _z_lru_val_cmp_f compare) { + cache->root = _z_lru_cache_delete_tree_node_rec(cache->root, _z_lru_cache_node_value(node), value_size, compare); +} +#else +_z_lru_cache_node_t *_z_lru_cache_search_list(_z_lru_cache_t *cache, void *value, _z_lru_val_cmp_f compare) { + if (cache->head == NULL) { + return NULL; + } + _z_lru_cache_node_data_t *node = cache->head; + // Parse list + while (node != NULL) { + _z_lru_cache_node_data_t *node_data = _z_lru_cache_node_data(node); + void *node_val = _z_lru_cache_node_value(node); + if (compare(node_val, value) == 0) { + return node; + } + node = node_data->next; + } + return NULL; +} +#endif + +// Main static functions +static void _z_lru_cache_delete_last(_z_lru_cache_t *cache, size_t value_size, _z_lru_val_cmp_f compare) { + _z_lru_cache_node_t *last = cache->tail; + assert(last != NULL); + _z_lru_cache_remove_list_node(cache, last); +#if Z_FEATURE_CACHE_TREE == 1 + _z_lru_cache_delete_tree_node(cache, last, value_size, compare); +#else + _ZP_UNUSED(compare); + _ZP_UNUSED(value_size); + z_free(last); +#endif + cache->len--; +} + +static void _z_lru_cache_insert_node(_z_lru_cache_t *cache, _z_lru_cache_node_t *node, _z_lru_val_cmp_f compare) { + _z_lru_cache_insert_list_node(cache, node); +#if Z_FEATURE_CACHE_TREE == 1 + _z_lru_cache_insert_tree_node(cache, node, compare); +#else + _ZP_UNUSED(compare); +#endif + cache->len++; +} + +static _z_lru_cache_node_t *_z_lru_cache_search_node(_z_lru_cache_t *cache, void *value, _z_lru_val_cmp_f compare) { +#if Z_FEATURE_CACHE_TREE == 1 + return _z_lru_cache_search_tree(cache, value, compare); +#else + return _z_lru_cache_search_list(cache, value, compare); +#endif +} + +// Public functions +_z_lru_cache_t _z_lru_cache_init(size_t capacity) { + _z_lru_cache_t cache = _z_lru_cache_null(); + cache.capacity = capacity; + return cache; +} + +void *_z_lru_cache_get(_z_lru_cache_t *cache, void *value, _z_lru_val_cmp_f compare) { + // Lookup tree if node exists. + _z_lru_cache_node_t *node = _z_lru_cache_search_node(cache, value, compare); + if (node == NULL) { + return NULL; + } + // Update list with node as most recent + _z_lru_cache_update_list(cache, node); + return _z_lru_cache_node_value(node); +} + +z_result_t _z_lru_cache_insert(_z_lru_cache_t *cache, void *value, size_t value_size, _z_lru_val_cmp_f compare) { + // Create node + _z_lru_cache_node_t *node = _z_lru_cache_node_create(value, value_size); + if (node == NULL) { + return _Z_ERR_SYSTEM_OUT_OF_MEMORY; + } + // Check capacity + if (cache->len == cache->capacity) { + // Delete lru entry + _z_lru_cache_delete_last(cache, value_size, compare); + } + // Update the cache + _z_lru_cache_insert_node(cache, node, compare); + return _Z_RES_OK; +} + +void _z_lru_cache_delete(_z_lru_cache_t *cache) { + _z_lru_cache_node_data_t *node = cache->head; + // Parse list + while (node != NULL) { + _z_lru_cache_node_t *tmp = node; + _z_lru_cache_node_data_t *node_data = _z_lru_cache_node_data(node); + node = node_data->next; + z_free(tmp); + } +} diff --git a/tests/z_lru_cache_test.c b/tests/z_lru_cache_test.c new file mode 100644 index 000000000..0cb8e5117 --- /dev/null +++ b/tests/z_lru_cache_test.c @@ -0,0 +1,213 @@ +// +// Copyright (c) 2024 ZettaScale Technology +// +// This program and the accompanying materials are made available under the +// terms of the Eclipse Public License 2.0 which is available at +// http://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 +// which is available at https://www.apache.org/licenses/LICENSE-2.0. +// +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// +// Contributors: +// ZettaScale Zenoh Team, +// + +#include +#include +#include +#include + +#include "zenoh-pico/collections/lru_cache.h" +#include "zenoh-pico/system/common/platform.h" + +#undef NDEBUG +#include + +#define CACHE_CAPACITY 10 + +typedef struct _dummy_t { + int foo; +} _dummy_t; + +int _dummy_compare(const void *first, const void *second) { + _dummy_t *d_first = (_dummy_t *)first; + _dummy_t *d_second = (_dummy_t *)second; + + if (d_first->foo == d_second->foo) { + return 0; + } else if (d_first->foo > d_second->foo) { + return 1; + } + return -1; +} + +_Z_LRU_CACHE_DEFINE(_dummy, _dummy_t, _dummy_compare) + +void print_tree_inorder(uint8_t *node) { + if (node == NULL) { + return; + } + // Traverse left + print_tree_inorder(*(uint8_t **)(node + 16)); + // Print node + printf("%d, %p\n", *(int *)(node + 32), node); + // Traverse right + print_tree_inorder(*(uint8_t **)(node + 24)); +} + +void print_list(uint8_t *node) { + while (node != NULL) { + printf("%d, %p\n", *(int *)(node + 32), node); + node = *(uint8_t **)(node + 8); + } +} + +void test_lru_init(void) { + _dummy_lru_cache_t dcache = _dummy_lru_cache_init(CACHE_CAPACITY); + assert(dcache.capacity == CACHE_CAPACITY); + assert(dcache.len == 0); + assert(dcache.head == NULL); + assert(dcache.tail == NULL); +#if Z_FEATURE_CACHE_TREE == 1 + assert(dcache.root == NULL); +#endif +} + +void test_lru_cache_insert(void) { + _dummy_lru_cache_t dcache = _dummy_lru_cache_init(CACHE_CAPACITY); + + _dummy_t v0 = {0}; + assert(_dummy_lru_cache_get(&dcache, &v0) == NULL); + assert(_dummy_lru_cache_insert(&dcache, &v0) == 0); + _dummy_t *res = _dummy_lru_cache_get(&dcache, &v0); + assert(res != NULL); + assert(res->foo == v0.foo); + + _dummy_t data[CACHE_CAPACITY] = {0}; + for (size_t i = 1; i < CACHE_CAPACITY; i++) { + data[i].foo = (int)i; + assert(_dummy_lru_cache_insert(&dcache, &data[i]) == 0); + } + for (size_t i = 0; i < CACHE_CAPACITY; i++) { + res = _dummy_lru_cache_get(&dcache, &data[i]); + assert(res != NULL); + assert(res->foo == data[i].foo); + } + _dummy_lru_cache_delete(&dcache); +} + +void test_lru_cache_deletion(void) { + _dummy_lru_cache_t dcache = _dummy_lru_cache_init(CACHE_CAPACITY); + + _dummy_t data[CACHE_CAPACITY + 1] = {0}; + for (size_t i = 0; i < CACHE_CAPACITY + 1; i++) { + data[i].foo = (int)i; + assert(_dummy_lru_cache_insert(&dcache, &data[i]) == 0); + } + // Check value deleted + assert(_dummy_lru_cache_get(&dcache, &data[0]) == NULL); + // Check remaining value + for (size_t i = 1; i < CACHE_CAPACITY + 1; i++) { + _dummy_t *res = _dummy_lru_cache_get(&dcache, &data[i]); + assert(res != NULL); + assert(res->foo == data[i].foo); + } + _dummy_lru_cache_delete(&dcache); +} + +void test_lru_cache_update(void) { + _dummy_lru_cache_t dcache = _dummy_lru_cache_init(CACHE_CAPACITY); + + _dummy_t data[CACHE_CAPACITY] = {0}; + for (size_t i = 0; i < CACHE_CAPACITY; i++) { + data[i].foo = (int)i; + assert(_dummy_lru_cache_insert(&dcache, &data[i]) == 0); + } + // Update value + assert(_dummy_lru_cache_get(&dcache, &data[0]) != NULL); + // Insert extra value + _dummy_t extra_data = {55}; + assert(_dummy_lru_cache_insert(&dcache, &extra_data) == 0); + // Check value deleted + assert(_dummy_lru_cache_get(&dcache, &data[1]) == NULL); + _dummy_lru_cache_delete(&dcache); +} + +static bool val_in_array(int val, int *array, size_t array_size) { + for (size_t i = 0; i < array_size; i++) { + if (val == array[i]) { + return true; + } + } + return false; +} + +void test_lru_cache_random_val(void) { + _dummy_lru_cache_t dcache = _dummy_lru_cache_init(10 * CACHE_CAPACITY); + + _dummy_t data[11 * CACHE_CAPACITY] = { + {1804289383}, {846930886}, {1681692777}, {1714636915}, {1957747793}, {424238335}, {719885386}, {1649760492}, + {596516649}, {1189641421}, {1025202362}, {1350490027}, {783368690}, {1102520059}, {2044897763}, {1967513926}, + {1365180540}, {1540383426}, {304089172}, {1303455736}, {35005211}, {521595368}, {294702567}, {1726956429}, + {336465782}, {861021530}, {278722862}, {233665123}, {2145174067}, {468703135}, {1101513929}, {1801979802}, + {1315634022}, {635723058}, {1369133069}, {1125898167}, {1059961393}, {2089018456}, {628175011}, {1656478042}, + {1131176229}, {1653377373}, {859484421}, {1914544919}, {608413784}, {756898537}, {1734575198}, {1973594324}, + {149798315}, {2038664370}, {1129566413}, {184803526}, {412776091}, {1424268980}, {1911759956}, {749241873}, + {137806862}, {42999170}, {982906996}, {135497281}, {511702305}, {2084420925}, {1937477084}, {1827336327}, + {572660336}, {1159126505}, {805750846}, {1632621729}, {1100661313}, {1433925857}, {1141616124}, {84353895}, + {939819582}, {2001100545}, {1998898814}, {1548233367}, {610515434}, {1585990364}, {1374344043}, {760313750}, + {1477171087}, {356426808}, {945117276}, {1889947178}, {1780695788}, {709393584}, {491705403}, {1918502651}, + {752392754}, {1474612399}, {2053999932}, {1264095060}, {1411549676}, {1843993368}, {943947739}, {1984210012}, + {855636226}, {1749698586}, {1469348094}, {1956297539}, {1036140795}, {463480570}, {2040651434}, {1975960378}, + {317097467}, {1892066601}, {1376710097}, {927612902}, {1330573317}, {603570492}, + }; + // Insert data + for (size_t i = 0; i < _ZP_ARRAY_SIZE(data); i++) { + assert(_dummy_lru_cache_insert(&dcache, &data[i]) == 0); + } + // Check values + for (size_t i = 0; i < _ZP_ARRAY_SIZE(data); i++) { + _dummy_t *res = _dummy_lru_cache_get(&dcache, &data[i]); + if (i < CACHE_CAPACITY) { + assert(res == NULL); + } else { + assert(res != NULL); + assert(res->foo == data[i].foo); + } + } + // Update most values + int not_upd_idx[CACHE_CAPACITY] = { + 34, 12, 42, 46, 56, 109, 103, 15, 96, 31, + }; + for (size_t i = CACHE_CAPACITY; i < _ZP_ARRAY_SIZE(data); i++) { + if (!val_in_array((int)i, not_upd_idx, _ZP_ARRAY_SIZE(not_upd_idx))) { + assert(_dummy_lru_cache_get(&dcache, &data[i]) != NULL); + } + } + // Insert back deleted value + for (size_t i = 0; i < CACHE_CAPACITY; i++) { + assert(_dummy_lru_cache_get(&dcache, &data[i]) == NULL); + assert(_dummy_lru_cache_insert(&dcache, &data[i]) == 0); + } + // Check deleted values + for (size_t i = 0; i < _ZP_ARRAY_SIZE(data); i++) { + _dummy_t *res = _dummy_lru_cache_get(&dcache, &data[i]); + if (val_in_array((int)i, not_upd_idx, _ZP_ARRAY_SIZE(not_upd_idx))) { + assert(res == NULL); + } else { + assert(res != NULL); + assert(res->foo == data[i].foo); + } + } + // Clean-up + _dummy_lru_cache_delete(&dcache); +} + +int main(void) { + test_lru_init(); + test_lru_cache_insert(); + test_lru_cache_deletion(); + test_lru_cache_update(); + test_lru_cache_random_val(); + return 0; +} From 773ba337ee52740a9ae25551feb0d92a5c88f8ae Mon Sep 17 00:00:00 2001 From: Jean-Roland Date: Wed, 18 Dec 2024 16:28:34 +0100 Subject: [PATCH 02/15] feat: add keyexpr_compare function --- include/zenoh-pico/collections/string.h | 1 + include/zenoh-pico/protocol/keyexpr.h | 2 +- src/collections/string.c | 7 +++++++ src/protocol/keyexpr.c | 17 +++++++++++++++++ 4 files changed, 26 insertions(+), 1 deletion(-) diff --git a/include/zenoh-pico/collections/string.h b/include/zenoh-pico/collections/string.h index 9e094b795..3c7535ab3 100644 --- a/include/zenoh-pico/collections/string.h +++ b/include/zenoh-pico/collections/string.h @@ -100,6 +100,7 @@ void _z_string_move_str(_z_string_t *dst, char *src); void _z_string_clear(_z_string_t *s); void _z_string_free(_z_string_t **s); void _z_string_reset(_z_string_t *s); +int _z_string_compare(const _z_string_t *left, const _z_string_t *right); bool _z_string_equals(const _z_string_t *left, const _z_string_t *right); _z_string_t _z_string_convert_bytes_le(const _z_slice_t *bs); _z_string_t _z_string_preallocate(const size_t len); diff --git a/include/zenoh-pico/protocol/keyexpr.h b/include/zenoh-pico/protocol/keyexpr.h index f6126a06d..a4f9e1acb 100644 --- a/include/zenoh-pico/protocol/keyexpr.h +++ b/include/zenoh-pico/protocol/keyexpr.h @@ -39,7 +39,7 @@ static inline _z_keyexpr_t _z_keyexpr_alias(const _z_keyexpr_t src) { ret._suffix = _z_string_alias(src._suffix); return ret; } - +int _z_keyexpr_compare(_z_keyexpr_t *first, _z_keyexpr_t *second); _z_keyexpr_t _z_keyexpr_from_string(uint16_t rid, _z_string_t *str); _z_keyexpr_t _z_keyexpr_from_substr(uint16_t rid, const char *str, size_t len); size_t _z_keyexpr_size(_z_keyexpr_t *p); diff --git a/src/collections/string.c b/src/collections/string.c index 236b85077..7cc72ce1f 100644 --- a/src/collections/string.c +++ b/src/collections/string.c @@ -101,6 +101,13 @@ void _z_string_free(_z_string_t **str) { } } +int _z_string_compare(const _z_string_t *left, const _z_string_t *right) { + if (_z_string_len(left) <= _z_string_len(right)) { + return strncmp(_z_string_data(left), _z_string_data(right), _z_string_len(left)); + } + return strncmp(_z_string_data(left), _z_string_data(right), _z_string_len(right)); +} + bool _z_string_equals(const _z_string_t *left, const _z_string_t *right) { if (_z_string_len(left) != _z_string_len(right)) { return false; diff --git a/src/protocol/keyexpr.c b/src/protocol/keyexpr.c index 5a4882566..70af6e8ee 100644 --- a/src/protocol/keyexpr.c +++ b/src/protocol/keyexpr.c @@ -21,6 +21,7 @@ #include "zenoh-pico/protocol/core.h" #include "zenoh-pico/utils/pointers.h" #include "zenoh-pico/utils/string.h" +#include "zenoh-pico/utils/logging.h" _z_keyexpr_t _z_rname(const char *rname) { return _z_rid_with_suffix(0, rname); } @@ -32,6 +33,22 @@ _z_keyexpr_t _z_rid_with_suffix(uint16_t rid, const char *suffix) { }; } +int _z_keyexpr_compare(_z_keyexpr_t *first, _z_keyexpr_t *second) { + if ((first->_id != 0) && (second->_id != 0)) { + if (first->_id == second->_id) { + return 0; + } else if (first->_id > second->_id) { + return 1; + } + return -1; + } + if (_z_keyexpr_has_suffix(first) && _z_keyexpr_has_suffix(second)) { + return _z_string_compare(&first->_suffix, &second->_suffix); + } + _Z_ERROR("Couldn't compare the two keyexpr"); + return -1; +} + _z_keyexpr_t _z_keyexpr_from_string(uint16_t rid, _z_string_t *str) { return (_z_keyexpr_t){ ._id = rid, From 438e63eec1eec372c9470aeccae6f6cb02f181e8 Mon Sep 17 00:00:00 2001 From: Jean-Roland Date: Wed, 18 Dec 2024 17:26:36 +0100 Subject: [PATCH 03/15] fix: clang format --- include/zenoh-pico/collections/lru_cache.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/include/zenoh-pico/collections/lru_cache.h b/include/zenoh-pico/collections/lru_cache.h index 5c46271ea..0f935d642 100644 --- a/include/zenoh-pico/collections/lru_cache.h +++ b/include/zenoh-pico/collections/lru_cache.h @@ -51,14 +51,14 @@ void *_z_lru_cache_get(_z_lru_cache_t *cache, void *value, _z_lru_val_cmp_f comp z_result_t _z_lru_cache_insert(_z_lru_cache_t *cache, void *value, size_t value_size, _z_lru_val_cmp_f compare); void _z_lru_cache_delete(_z_lru_cache_t *cache); -#define _Z_LRU_CACHE_DEFINE(name, type, compare_f) \ +#define _Z_LRU_CACHE_DEFINE(name, type, compare_f) \ typedef _z_lru_cache_t name##_lru_cache_t; \ static inline name##_lru_cache_t name##_lru_cache_init(size_t capacity) { return _z_lru_cache_init(capacity); } \ static inline type *name##_lru_cache_get(name##_lru_cache_t *cache, type *val) { \ - return (type *)_z_lru_cache_get(cache, (void *)val, compare_f); \ + return (type *)_z_lru_cache_get(cache, (void *)val, compare_f); \ } \ static inline z_result_t name##_lru_cache_insert(name##_lru_cache_t *cache, type *val) { \ - return _z_lru_cache_insert(cache, (void *)val, sizeof(type), compare_f); \ + return _z_lru_cache_insert(cache, (void *)val, sizeof(type), compare_f); \ } \ static inline void name##_lru_cache_delete(name##_lru_cache_t *cache) { _z_lru_cache_delete(cache); } From b956608f1ac7abe07af3332bed32fc3ce3e4cff7 Mon Sep 17 00:00:00 2001 From: Jean-Roland Date: Wed, 18 Dec 2024 17:27:22 +0100 Subject: [PATCH 04/15] feat: add rx cache size config token --- include/zenoh-pico/config.h | 5 +++++ include/zenoh-pico/config.h.in | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/include/zenoh-pico/config.h b/include/zenoh-pico/config.h index 7ec06acdf..3542c7d1d 100644 --- a/include/zenoh-pico/config.h +++ b/include/zenoh-pico/config.h @@ -175,6 +175,11 @@ */ #define Z_IOSLICE_SIZE 128 +/** + * Default size for the rx cache size (if activated). + */ +#define Z_RX_CACHE_SIZE 10 + /** * Default get timeout in milliseconds. */ diff --git a/include/zenoh-pico/config.h.in b/include/zenoh-pico/config.h.in index 92b7006ea..a3748c432 100644 --- a/include/zenoh-pico/config.h.in +++ b/include/zenoh-pico/config.h.in @@ -175,6 +175,11 @@ */ #define Z_IOSLICE_SIZE 128 +/** + * Default size for the rx cache size (if activated). + */ +#define Z_RX_CACHE_SIZE 10 + /** * Default get timeout in milliseconds. */ From 6b49be8b3f9bb69ca9760b2b3749112285234254 Mon Sep 17 00:00:00 2001 From: Jean-Roland Date: Wed, 18 Dec 2024 17:29:39 +0100 Subject: [PATCH 05/15] refactor: pass keyexpr as pointer --- include/zenoh-pico/net/primitives.h | 4 ++-- include/zenoh-pico/net/query.h | 4 ++-- include/zenoh-pico/protocol/core.h | 2 +- include/zenoh-pico/protocol/keyexpr.h | 9 ++++----- include/zenoh-pico/session/resource.h | 2 +- src/api/api.c | 16 ++++++++-------- src/net/liveliness.c | 2 +- src/net/primitives.c | 8 ++++---- src/protocol/core.c | 6 +++--- src/protocol/keyexpr.c | 2 +- src/session/interest.c | 6 +++--- src/session/liveliness.c | 2 +- src/session/queryable.c | 2 +- src/session/resource.c | 19 ++++++++++--------- src/session/rx.c | 2 +- src/session/subscription.c | 6 +++--- 16 files changed, 46 insertions(+), 46 deletions(-) diff --git a/include/zenoh-pico/net/primitives.h b/include/zenoh-pico/net/primitives.h index b53a8f4e4..556dcdc7b 100644 --- a/include/zenoh-pico/net/primitives.h +++ b/include/zenoh-pico/net/primitives.h @@ -59,7 +59,7 @@ void _z_scout(const z_what_t what, const _z_id_t zid, _z_string_t *locator, cons * Returns: * A numerical id of the declared resource. */ -uint16_t _z_declare_resource(_z_session_t *zn, _z_keyexpr_t keyexpr); +uint16_t _z_declare_resource(_z_session_t *zn, const _z_keyexpr_t *keyexpr); /** * Associate a numerical id with the given resource key. @@ -218,7 +218,7 @@ z_result_t _z_undeclare_queryable(_z_queryable_t *qle); * kind: The type of operation. * attachment: An optional attachment to the reply. */ -z_result_t _z_send_reply(const _z_query_t *query, const _z_session_rc_t *zsrc, const _z_keyexpr_t keyexpr, +z_result_t _z_send_reply(const _z_query_t *query, const _z_session_rc_t *zsrc, const _z_keyexpr_t *keyexpr, const _z_value_t payload, const z_sample_kind_t kind, const z_congestion_control_t cong_ctrl, z_priority_t priority, bool is_express, const _z_timestamp_t *timestamp, const _z_bytes_t attachment); diff --git a/include/zenoh-pico/net/query.h b/include/zenoh-pico/net/query.h index 2a392b987..675264744 100644 --- a/include/zenoh-pico/net/query.h +++ b/include/zenoh-pico/net/query.h @@ -67,8 +67,8 @@ static inline _z_query_t _z_query_alias(_z_value_t *value, _z_keyexpr_t *key, co _z_session_rc_t *zn, uint32_t request_id, const _z_bytes_t *attachment, bool anyke) { _z_query_t ret; - ret._key = _z_keyexpr_alias(*key); - ret._value = _z_value_alias(*value); + ret._key = _z_keyexpr_alias(key); + ret._value = _z_value_alias(value); ret._request_id = request_id; ret._zn = _z_session_rc_clone_as_weak(zn); ret._attachment = _z_bytes_alias(*attachment); diff --git a/include/zenoh-pico/protocol/core.h b/include/zenoh-pico/protocol/core.h index 0d98b5b4f..7f30f9c5f 100644 --- a/include/zenoh-pico/protocol/core.h +++ b/include/zenoh-pico/protocol/core.h @@ -181,7 +181,7 @@ static inline bool _z_value_check(const _z_value_t *value) { } _z_value_t _z_value_steal(_z_value_t *value); z_result_t _z_value_copy(_z_value_t *dst, const _z_value_t *src); -_z_value_t _z_value_alias(_z_value_t src); +_z_value_t _z_value_alias(_z_value_t *src); void _z_value_move(_z_value_t *dst, _z_value_t *src); void _z_value_clear(_z_value_t *src); void _z_value_free(_z_value_t **hello); diff --git a/include/zenoh-pico/protocol/keyexpr.h b/include/zenoh-pico/protocol/keyexpr.h index a4f9e1acb..d80d3b732 100644 --- a/include/zenoh-pico/protocol/keyexpr.h +++ b/include/zenoh-pico/protocol/keyexpr.h @@ -32,11 +32,11 @@ bool _z_keyexpr_suffix_equals(const _z_keyexpr_t *left, const _z_keyexpr_t *righ /*------------------ clone/Copy/Free helpers ------------------*/ // Warning: None of the sub-types require a non-0 initialization. Add a init function if it changes. static inline _z_keyexpr_t _z_keyexpr_null(void) { return (_z_keyexpr_t){0}; } -static inline _z_keyexpr_t _z_keyexpr_alias(const _z_keyexpr_t src) { +static inline _z_keyexpr_t _z_keyexpr_alias(const _z_keyexpr_t *src) { _z_keyexpr_t ret; - ret._id = src._id; - ret._mapping = src._mapping; - ret._suffix = _z_string_alias(src._suffix); + ret._id = src->_id; + ret._mapping = src->_mapping; + ret._suffix = _z_string_alias(src->_suffix); return ret; } int _z_keyexpr_compare(_z_keyexpr_t *first, _z_keyexpr_t *second); @@ -46,7 +46,6 @@ size_t _z_keyexpr_size(_z_keyexpr_t *p); z_result_t _z_keyexpr_copy(_z_keyexpr_t *dst, const _z_keyexpr_t *src); _z_keyexpr_t _z_keyexpr_duplicate(const _z_keyexpr_t *src); _z_keyexpr_t *_z_keyexpr_clone(const _z_keyexpr_t *src); -_z_keyexpr_t _z_keyexpr_alias(_z_keyexpr_t src); /// Returns either keyexpr defined by id + mapping with null suffix if try_declared is true and id is non-zero, /// or keyexpr defined by its suffix only, with 0 id and no mapping. This is to be used only when forwarding /// keyexpr in user api to properly separate declared keyexpr from its suffix. diff --git a/include/zenoh-pico/session/resource.h b/include/zenoh-pico/session/resource.h index 61af9989b..a94933643 100644 --- a/include/zenoh-pico/session/resource.h +++ b/include/zenoh-pico/session/resource.h @@ -31,7 +31,7 @@ uint16_t _z_get_resource_id(_z_session_t *zn); _z_resource_t *_z_get_resource_by_id(_z_session_t *zn, uint16_t mapping, _z_zint_t rid); _z_resource_t *_z_get_resource_by_key(_z_session_t *zn, const _z_keyexpr_t *keyexpr); _z_keyexpr_t _z_get_expanded_key_from_key(_z_session_t *zn, const _z_keyexpr_t *keyexpr); -uint16_t _z_register_resource(_z_session_t *zn, const _z_keyexpr_t key, uint16_t id, uint16_t register_to_mapping); +uint16_t _z_register_resource(_z_session_t *zn, const _z_keyexpr_t *key, uint16_t id, uint16_t register_to_mapping); void _z_unregister_resource(_z_session_t *zn, uint16_t id, uint16_t mapping); void _z_unregister_resources_for_peer(_z_session_t *zn, uint16_t mapping); void _z_flush_resources(_z_session_t *zn); diff --git a/src/api/api.c b/src/api/api.c index 207f4b467..334ccfee2 100644 --- a/src/api/api.c +++ b/src/api/api.c @@ -942,7 +942,7 @@ z_result_t z_declare_publisher(const z_loaned_session_t *zs, z_owned_publisher_t if (_Z_RC_IN_VAL(zs)->_tp._type == _Z_TRANSPORT_UNICAST_TYPE) { _z_resource_t *r = _z_get_resource_by_key(_Z_RC_IN_VAL(zs), &keyexpr_aliased); if (r == NULL) { - uint16_t id = _z_declare_resource(_Z_RC_IN_VAL(zs), keyexpr_aliased); + uint16_t id = _z_declare_resource(_Z_RC_IN_VAL(zs), &keyexpr_aliased); key = _z_keyexpr_from_string(id, &keyexpr_aliased._suffix); } } @@ -1215,7 +1215,7 @@ z_result_t z_declare_queryable(const z_loaned_session_t *zs, z_owned_queryable_t if (_Z_RC_IN_VAL(zs)->_tp._type == _Z_TRANSPORT_UNICAST_TYPE) { _z_resource_t *r = _z_get_resource_by_key(_Z_RC_IN_VAL(zs), &keyexpr_aliased); if (r == NULL) { - uint16_t id = _z_declare_resource(_Z_RC_IN_VAL(zs), keyexpr_aliased); + uint16_t id = _z_declare_resource(_Z_RC_IN_VAL(zs), &keyexpr_aliased); key = _z_rid_with_suffix(id, NULL); } } @@ -1267,7 +1267,7 @@ z_result_t z_query_reply(const z_loaned_query_t *query, const z_loaned_keyexpr_t _z_value_t value = {.payload = _z_bytes_from_owned_bytes(&payload->_this), .encoding = _z_encoding_from_owned(&opts.encoding->_this)}; - z_result_t ret = _z_send_reply(_Z_RC_IN_VAL(query), &sess_rc, keyexpr_aliased, value, Z_SAMPLE_KIND_PUT, + z_result_t ret = _z_send_reply(_Z_RC_IN_VAL(query), &sess_rc, &keyexpr_aliased, value, Z_SAMPLE_KIND_PUT, opts.congestion_control, opts.priority, opts.is_express, opts.timestamp, _z_bytes_from_owned_bytes(&opts.attachment->_this)); z_bytes_drop(payload); @@ -1303,7 +1303,7 @@ z_result_t z_query_reply_del(const z_loaned_query_t *query, const z_loaned_keyex _z_value_t value = {.payload = _z_bytes_null(), .encoding = _z_encoding_null()}; - z_result_t ret = _z_send_reply(_Z_RC_IN_VAL(query), &sess_rc, keyexpr_aliased, value, Z_SAMPLE_KIND_DELETE, + z_result_t ret = _z_send_reply(_Z_RC_IN_VAL(query), &sess_rc, &keyexpr_aliased, value, Z_SAMPLE_KIND_DELETE, opts.congestion_control, opts.priority, opts.is_express, opts.timestamp, _z_bytes_from_owned_bytes(&opts.attachment->_this)); // Clean-up @@ -1377,7 +1377,7 @@ z_result_t z_keyexpr_from_substr(z_owned_keyexpr_t *key, const char *name, size_ z_result_t z_declare_keyexpr(const z_loaned_session_t *zs, z_owned_keyexpr_t *key, const z_loaned_keyexpr_t *keyexpr) { _z_keyexpr_t k = _z_keyexpr_alias_from_user_defined(*keyexpr, false); - uint16_t id = _z_declare_resource(_Z_RC_IN_VAL(zs), k); + uint16_t id = _z_declare_resource(_Z_RC_IN_VAL(zs), &k); key->_val = _z_rid_with_suffix(id, NULL); // we still need to store the original suffix, for user needs // (for example to compare keys or perform other operations on their string representation). @@ -1426,7 +1426,7 @@ z_result_t z_declare_subscriber(const z_loaned_session_t *zs, z_owned_subscriber callback->_this._val.context = NULL; _z_keyexpr_t keyexpr_aliased = _z_keyexpr_alias_from_user_defined(*keyexpr, true); - _z_keyexpr_t key = _z_keyexpr_alias(keyexpr_aliased); + _z_keyexpr_t key = _z_keyexpr_alias(&keyexpr_aliased); // TODO: Currently, if resource declarations are done over multicast transports, the current protocol definition // lacks a way to convey them to later-joining nodes. Thus, in the current version automatic @@ -1435,7 +1435,7 @@ z_result_t z_declare_subscriber(const z_loaned_session_t *zs, z_owned_subscriber _z_resource_t *r = _z_get_resource_by_key(_Z_RC_IN_VAL(zs), &keyexpr_aliased); if (r == NULL) { bool do_keydecl = true; - _z_keyexpr_t resource_key = _z_keyexpr_alias(keyexpr_aliased); + _z_keyexpr_t resource_key = _z_keyexpr_alias(&keyexpr_aliased); // Remove wild char *wild = _z_string_pbrk(&keyexpr_aliased._suffix, "*$"); if ((wild != NULL) && _z_keyexpr_has_suffix(&keyexpr_aliased)) { @@ -1450,7 +1450,7 @@ z_result_t z_declare_subscriber(const z_loaned_session_t *zs, z_owned_subscriber } // Declare resource if (do_keydecl) { - uint16_t id = _z_declare_resource(_Z_RC_IN_VAL(zs), resource_key); + uint16_t id = _z_declare_resource(_Z_RC_IN_VAL(zs), &resource_key); key = _z_rid_with_suffix(id, wild); } _z_keyexpr_clear(&resource_key); diff --git a/src/net/liveliness.c b/src/net/liveliness.c index 5348bcbb1..b827aada6 100644 --- a/src/net/liveliness.c +++ b/src/net/liveliness.c @@ -149,7 +149,7 @@ z_result_t _z_liveliness_query(_z_session_t *zn, const _z_keyexpr_t *keyexpr, _z ret = _z_liveliness_register_pending_query(zn, id, pq); if (ret == _Z_RES_OK) { _ZP_UNUSED(timeout_ms); // Current interest in pico don't support timeout - _z_keyexpr_t key = _z_keyexpr_alias(*keyexpr); + _z_keyexpr_t key = _z_keyexpr_alias(keyexpr); _z_interest_t interest = _z_make_interest(&key, id, _Z_INTEREST_FLAG_KEYEXPRS | _Z_INTEREST_FLAG_TOKENS | _Z_INTEREST_FLAG_RESTRICTED | _Z_INTEREST_FLAG_CURRENT); diff --git a/src/net/primitives.c b/src/net/primitives.c index b7651bfb2..e6b9a73d6 100644 --- a/src/net/primitives.c +++ b/src/net/primitives.c @@ -86,7 +86,7 @@ void _z_scout(const z_what_t what, const _z_id_t zid, _z_string_t *locator, cons } /*------------------ Resource Declaration ------------------*/ -uint16_t _z_declare_resource(_z_session_t *zn, _z_keyexpr_t keyexpr) { +uint16_t _z_declare_resource(_z_session_t *zn, const _z_keyexpr_t *keyexpr) { uint16_t ret = Z_RESOURCE_ID_NONE; // FIXME: remove this check when resource declaration is implemented for multicast transport @@ -145,7 +145,7 @@ _z_keyexpr_t _z_update_keyexpr_to_declared(_z_session_t *zs, _z_keyexpr_t keyexp if (r != NULL) { key = _z_rid_with_suffix(r->_id, NULL); } else { - uint16_t id = _z_declare_resource(zs, keyexpr_aliased); + uint16_t id = _z_declare_resource(zs, &keyexpr_aliased); key = _z_rid_with_suffix(id, NULL); } } @@ -365,7 +365,7 @@ z_result_t _z_undeclare_queryable(_z_queryable_t *qle) { return _Z_RES_OK; } -z_result_t _z_send_reply(const _z_query_t *query, const _z_session_rc_t *zsrc, _z_keyexpr_t keyexpr, +z_result_t _z_send_reply(const _z_query_t *query, const _z_session_rc_t *zsrc, const _z_keyexpr_t *keyexpr, const _z_value_t payload, const z_sample_kind_t kind, const z_congestion_control_t cong_ctrl, z_priority_t priority, bool is_express, const _z_timestamp_t *timestamp, const _z_bytes_t att) { @@ -376,7 +376,7 @@ z_result_t _z_send_reply(const _z_query_t *query, const _z_session_rc_t *zsrc, _ _z_keyexpr_t r_ke; if (query->_anyke == false) { q_ke = _z_get_expanded_key_from_key(zn, &query->_key); - r_ke = _z_get_expanded_key_from_key(zn, &keyexpr); + r_ke = _z_get_expanded_key_from_key(zn, keyexpr); if (_z_keyexpr_suffix_intersects(&q_ke, &r_ke) == false) { ret = _Z_ERR_KEYEXPR_NOT_MATCH; } diff --git a/src/protocol/core.c b/src/protocol/core.c index 37c2a7bcc..410b3a730 100644 --- a/src/protocol/core.c +++ b/src/protocol/core.c @@ -62,10 +62,10 @@ z_result_t _z_value_copy(_z_value_t *dst, const _z_value_t *src) { _Z_CLEAN_RETURN_IF_ERR(_z_bytes_copy(&dst->payload, &src->payload), _z_encoding_clear(&dst->encoding)); return _Z_RES_OK; } -_z_value_t _z_value_alias(_z_value_t src) { +_z_value_t _z_value_alias(_z_value_t *src) { _z_value_t dst; - dst.payload = _z_bytes_alias(src.payload); - dst.encoding = _z_encoding_alias(src.encoding); + dst.payload = _z_bytes_alias(src->payload); + dst.encoding = _z_encoding_alias(src->encoding); return dst; } diff --git a/src/protocol/keyexpr.c b/src/protocol/keyexpr.c index 70af6e8ee..35d8a5f5c 100644 --- a/src/protocol/keyexpr.c +++ b/src/protocol/keyexpr.c @@ -19,9 +19,9 @@ #include #include "zenoh-pico/protocol/core.h" +#include "zenoh-pico/utils/logging.h" #include "zenoh-pico/utils/pointers.h" #include "zenoh-pico/utils/string.h" -#include "zenoh-pico/utils/logging.h" _z_keyexpr_t _z_rname(const char *rname) { return _z_rid_with_suffix(0, rname); } diff --git a/src/session/interest.c b/src/session/interest.c index 1ef8f033b..8aeb2a286 100644 --- a/src/session/interest.c +++ b/src/session/interest.c @@ -104,7 +104,7 @@ static z_result_t _z_interest_send_decl_resource(_z_session_t *zn, uint32_t inte while (xs != NULL) { _z_resource_t *res = _z_resource_list_head(xs); // Build the declare message to send on the wire - _z_keyexpr_t key = _z_keyexpr_alias(res->_key); + _z_keyexpr_t key = _z_keyexpr_alias(&res->_key); _z_declaration_t declaration = _z_make_decl_keyexpr(res->_id, &key); _z_network_message_t n_msg = _z_n_msg_make_declare(declaration, true, interest_id); if (_z_send_n_msg(zn, &n_msg, Z_RELIABILITY_RELIABLE, Z_CONGESTION_CONTROL_BLOCK) != _Z_RES_OK) { @@ -126,7 +126,7 @@ static z_result_t _z_interest_send_decl_subscriber(_z_session_t *zn, uint32_t in while (xs != NULL) { _z_subscription_rc_t *sub = _z_subscription_rc_list_head(xs); // Build the declare message to send on the wire - _z_keyexpr_t key = _z_keyexpr_alias(_Z_RC_IN_VAL(sub)->_key); + _z_keyexpr_t key = _z_keyexpr_alias(&_Z_RC_IN_VAL(sub)->_key); _z_declaration_t declaration = _z_make_decl_subscriber(&key, _Z_RC_IN_VAL(sub)->_id); _z_network_message_t n_msg = _z_n_msg_make_declare(declaration, true, interest_id); if (_z_send_n_msg(zn, &n_msg, Z_RELIABILITY_RELIABLE, Z_CONGESTION_CONTROL_BLOCK) != _Z_RES_OK) { @@ -155,7 +155,7 @@ static z_result_t _z_interest_send_decl_queryable(_z_session_t *zn, uint32_t int while (xs != NULL) { _z_session_queryable_rc_t *qle = _z_session_queryable_rc_list_head(xs); // Build the declare message to send on the wire - _z_keyexpr_t key = _z_keyexpr_alias(_Z_RC_IN_VAL(qle)->_key); + _z_keyexpr_t key = _z_keyexpr_alias(&_Z_RC_IN_VAL(qle)->_key); _z_declaration_t declaration = _z_make_decl_queryable( &key, _Z_RC_IN_VAL(qle)->_id, _Z_RC_IN_VAL(qle)->_complete, _Z_QUERYABLE_DISTANCE_DEFAULT); _z_network_message_t n_msg = _z_n_msg_make_declare(declaration, true, interest_id); diff --git a/src/session/liveliness.c b/src/session/liveliness.c index dcf2bd0ac..f3e7f0aa0 100644 --- a/src/session/liveliness.c +++ b/src/session/liveliness.c @@ -85,7 +85,7 @@ z_result_t _z_liveliness_subscription_declare(_z_session_t *zn, uint32_t id, con _z_session_mutex_unlock(zn); if (ret == _Z_RES_OK) { - _z_keyexpr_t key = _z_keyexpr_alias(*keyexpr); + _z_keyexpr_t key = _z_keyexpr_alias(keyexpr); ret = _z_trigger_liveliness_subscriptions_declare(zn, &key, timestamp); } diff --git a/src/session/queryable.c b/src/session/queryable.c index cc1ef0b82..8b84a03f3 100644 --- a/src/session/queryable.c +++ b/src/session/queryable.c @@ -38,7 +38,7 @@ static inline bool _z_queryable_get_from_cache(_z_session_t *zn, const _z_keyexp if (!_z_keyexpr_equals(ke, &zn->_queryable_cache.ke_in)) { return false; } - *ke_val = _z_keyexpr_alias(zn->_queryable_cache.ke_out); + *ke_val = _z_keyexpr_alias(&zn->_queryable_cache.ke_out); *infos_val = _z_queryable_infos_svec_alias(&zn->_queryable_cache.infos); *qle_nb = zn->_queryable_cache.qle_nb; return true; diff --git a/src/session/resource.c b/src/session/resource.c index b95b4c850..a719cb6b6 100644 --- a/src/session/resource.c +++ b/src/session/resource.c @@ -103,7 +103,7 @@ static _z_keyexpr_t __z_get_expanded_key_from_key(_z_resource_list_t *xs, const } // Keyexpr can be aliased from a rx buffer if (force_alias) { - return _z_keyexpr_alias(*keyexpr); + return _z_keyexpr_alias(keyexpr); } else { return _z_keyexpr_duplicate(keyexpr); } @@ -218,29 +218,30 @@ _z_keyexpr_t _z_get_expanded_key_from_key(_z_session_t *zn, const _z_keyexpr_t * } /// Returns the ID of the registered keyexpr. Returns 0 if registration failed. -uint16_t _z_register_resource(_z_session_t *zn, _z_keyexpr_t key, uint16_t id, uint16_t register_to_mapping) { +uint16_t _z_register_resource(_z_session_t *zn, const _z_keyexpr_t *key, uint16_t id, uint16_t register_to_mapping) { uint16_t ret = Z_RESOURCE_ID_NONE; uint16_t mapping = register_to_mapping; - uint16_t parent_mapping = _z_keyexpr_mapping_id(&key); + uint16_t parent_mapping = _z_keyexpr_mapping_id(key); + _z_keyexpr_t full_ke = _z_keyexpr_alias(key); _z_session_mutex_lock(zn); - if (key._id != Z_RESOURCE_ID_NONE) { + if (key->_id != Z_RESOURCE_ID_NONE) { if (parent_mapping == mapping) { - _z_resource_t *parent = __unsafe_z_get_resource_by_id(zn, parent_mapping, key._id); + _z_resource_t *parent = __unsafe_z_get_resource_by_id(zn, parent_mapping, key->_id); parent->_refcount++; } else { - key = __unsafe_z_get_expanded_key_from_key(zn, &key, false); + full_ke = __unsafe_z_get_expanded_key_from_key(zn, key, false); } } - ret = key._id; - if (_z_keyexpr_has_suffix(&key)) { + ret = full_ke._id; + if (_z_keyexpr_has_suffix(&full_ke)) { _z_resource_t *res = z_malloc(sizeof(_z_resource_t)); if (res == NULL) { ret = Z_RESOURCE_ID_NONE; } else { res->_refcount = 1; - res->_key = _z_keyexpr_duplicate(&key); + res->_key = _z_keyexpr_duplicate(&full_ke); ret = id == Z_RESOURCE_ID_NONE ? _z_get_resource_id(zn) : id; res->_id = ret; // Register the resource diff --git a/src/session/rx.c b/src/session/rx.c index 9d44fcc45..da0c1ddef 100644 --- a/src/session/rx.c +++ b/src/session/rx.c @@ -46,7 +46,7 @@ z_result_t _z_handle_network_message(_z_session_rc_t *zsrc, _z_zenoh_message_t * _Z_DEBUG("Handling _Z_N_DECLARE: %i", decl->_decl._tag); switch (decl->_decl._tag) { case _Z_DECL_KEXPR: { - if (_z_register_resource(zn, decl->_decl._body._decl_kexpr._keyexpr, + if (_z_register_resource(zn, &decl->_decl._body._decl_kexpr._keyexpr, decl->_decl._body._decl_kexpr._id, local_peer_id) == 0) { ret = _Z_ERR_ENTITY_DECLARATION_FAILED; } diff --git a/src/session/subscription.c b/src/session/subscription.c index f614ddf0e..8ec1027aa 100644 --- a/src/session/subscription.c +++ b/src/session/subscription.c @@ -39,7 +39,7 @@ static inline bool _z_subscription_get_from_cache(_z_session_t *zn, const _z_key if (!_z_keyexpr_equals(ke, &zn->_subscription_cache.ke_in)) { return false; } - *ke_val = _z_keyexpr_alias(zn->_subscription_cache.ke_out); + *ke_val = _z_keyexpr_alias(&zn->_subscription_cache.ke_out); *infos_val = _z_subscription_infos_svec_alias(&zn->_subscription_cache.infos); *sub_nb = zn->_subscription_cache.sub_nb; return true; @@ -202,7 +202,7 @@ z_result_t _z_trigger_liveliness_subscriptions_declare(_z_session_t *zn, _z_keye _z_encoding_t encoding = _z_encoding_null(); _z_bytes_t payload = _z_bytes_null(); _z_bytes_t attachment = _z_bytes_null(); - _z_keyexpr_t key = _z_keyexpr_alias(*keyexpr); + _z_keyexpr_t key = _z_keyexpr_alias(keyexpr); return _z_trigger_subscriptions_impl(zn, _Z_SUBSCRIBER_KIND_LIVELINESS_SUBSCRIBER, &key, &payload, &encoding, Z_SAMPLE_KIND_PUT, timestamp, _Z_N_QOS_DEFAULT, &attachment, Z_RELIABILITY_RELIABLE); @@ -213,7 +213,7 @@ z_result_t _z_trigger_liveliness_subscriptions_undeclare(_z_session_t *zn, _z_ke _z_encoding_t encoding = _z_encoding_null(); _z_bytes_t payload = _z_bytes_null(); _z_bytes_t attachment = _z_bytes_null(); - _z_keyexpr_t key = _z_keyexpr_alias(*keyexpr); + _z_keyexpr_t key = _z_keyexpr_alias(keyexpr); return _z_trigger_subscriptions_impl(zn, _Z_SUBSCRIBER_KIND_LIVELINESS_SUBSCRIBER, &key, &payload, &encoding, Z_SAMPLE_KIND_DELETE, timestamp, _Z_N_QOS_DEFAULT, &attachment, Z_RELIABILITY_RELIABLE); From 4b113e43bdff153d84136be081bba39db87e37d0 Mon Sep 17 00:00:00 2001 From: Jean-Roland Date: Wed, 18 Dec 2024 17:42:53 +0100 Subject: [PATCH 06/15] fix: remove debug functions --- tests/z_lru_cache_test.c | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/tests/z_lru_cache_test.c b/tests/z_lru_cache_test.c index 0cb8e5117..b556d4584 100644 --- a/tests/z_lru_cache_test.c +++ b/tests/z_lru_cache_test.c @@ -43,25 +43,6 @@ int _dummy_compare(const void *first, const void *second) { _Z_LRU_CACHE_DEFINE(_dummy, _dummy_t, _dummy_compare) -void print_tree_inorder(uint8_t *node) { - if (node == NULL) { - return; - } - // Traverse left - print_tree_inorder(*(uint8_t **)(node + 16)); - // Print node - printf("%d, %p\n", *(int *)(node + 32), node); - // Traverse right - print_tree_inorder(*(uint8_t **)(node + 24)); -} - -void print_list(uint8_t *node) { - while (node != NULL) { - printf("%d, %p\n", *(int *)(node + 32), node); - node = *(uint8_t **)(node + 8); - } -} - void test_lru_init(void) { _dummy_lru_cache_t dcache = _dummy_lru_cache_init(CACHE_CAPACITY); assert(dcache.capacity == CACHE_CAPACITY); From 908bf18bf76d09b10be82fdb6b8e962e64855cb9 Mon Sep 17 00:00:00 2001 From: Jean-Roland Date: Thu, 19 Dec 2024 11:57:06 +0100 Subject: [PATCH 07/15] feat: use lru cache for subscription cache --- include/zenoh-pico/net/session.h | 2 +- include/zenoh-pico/session/subscription.h | 7 +- src/collections/lru_cache.c | 1 + src/session/subscription.c | 130 +++++++++------------- src/session/utils.c | 4 +- 5 files changed, 63 insertions(+), 81 deletions(-) diff --git a/include/zenoh-pico/net/session.h b/include/zenoh-pico/net/session.h index 4babe6771..463499cc9 100644 --- a/include/zenoh-pico/net/session.h +++ b/include/zenoh-pico/net/session.h @@ -69,7 +69,7 @@ typedef struct _z_session_t { _z_subscription_rc_list_t *_subscriptions; _z_subscription_rc_list_t *_liveliness_subscriptions; #if Z_FEATURE_RX_CACHE == 1 - _z_subscription_cache_t _subscription_cache; + _z_subscription_lru_cache_t _subscription_cache; #endif #endif diff --git a/include/zenoh-pico/session/subscription.h b/include/zenoh-pico/session/subscription.h index b70871b30..a78d23fd8 100644 --- a/include/zenoh-pico/session/subscription.h +++ b/include/zenoh-pico/session/subscription.h @@ -15,6 +15,7 @@ #ifndef INCLUDE_ZENOH_PICO_SESSION_SUBSCRIPTION_H #define INCLUDE_ZENOH_PICO_SESSION_SUBSCRIPTION_H +#include "zenoh-pico/collections/lru_cache.h" #include "zenoh-pico/net/encoding.h" #include "zenoh-pico/protocol/core.h" #include "zenoh-pico/session/session.h" @@ -40,7 +41,9 @@ typedef struct { _z_keyexpr_t ke_out; _z_subscription_infos_svec_t infos; size_t sub_nb; -} _z_subscription_cache_t; +} _z_subscription_cache_data_t; + +int _z_subscription_cache_data_compare(const void *first, const void *second); /*------------------ Subscription ------------------*/ z_result_t _z_trigger_subscriptions_put(_z_session_t *zn, _z_keyexpr_t *keyexpr, _z_bytes_t *payload, @@ -59,7 +62,7 @@ z_result_t _z_trigger_liveliness_subscriptions_undeclare(_z_session_t *zn, _z_ke #if Z_FEATURE_SUBSCRIPTION == 1 #if Z_FEATURE_RX_CACHE == 1 -void _z_subscription_cache_clear(_z_subscription_cache_t *cache); +_Z_LRU_CACHE_DEFINE(_z_subscription, _z_subscription_cache_data_t, _z_subscription_cache_data_compare) #endif _z_subscription_rc_t *_z_get_subscription_by_id(_z_session_t *zn, _z_subscriber_kind_t kind, const _z_zint_t id); diff --git a/src/collections/lru_cache.c b/src/collections/lru_cache.c index 0bbcbf381..81505755e 100644 --- a/src/collections/lru_cache.c +++ b/src/collections/lru_cache.c @@ -297,6 +297,7 @@ void *_z_lru_cache_get(_z_lru_cache_t *cache, void *value, _z_lru_val_cmp_f comp } z_result_t _z_lru_cache_insert(_z_lru_cache_t *cache, void *value, size_t value_size, _z_lru_val_cmp_f compare) { + assert(cache->capacity > 0); // Create node _z_lru_cache_node_t *node = _z_lru_cache_node_create(value, value_size); if (node == NULL) { diff --git a/src/session/subscription.c b/src/session/subscription.c index 8ec1027aa..d4c7ffe40 100644 --- a/src/session/subscription.c +++ b/src/session/subscription.c @@ -33,57 +33,19 @@ #define _Z_SUBINFOS_VEC_SIZE 4 // Arbitrary initial size -#if Z_FEATURE_RX_CACHE == 1 -static inline bool _z_subscription_get_from_cache(_z_session_t *zn, const _z_keyexpr_t *ke, _z_keyexpr_t *ke_val, - _z_subscription_infos_svec_t *infos_val, size_t *sub_nb) { - if (!_z_keyexpr_equals(ke, &zn->_subscription_cache.ke_in)) { - return false; - } - *ke_val = _z_keyexpr_alias(&zn->_subscription_cache.ke_out); - *infos_val = _z_subscription_infos_svec_alias(&zn->_subscription_cache.infos); - *sub_nb = zn->_subscription_cache.sub_nb; - return true; -} - -static inline void _z_subscription_update_cache(_z_session_t *zn, const _z_keyexpr_t *ke_in, const _z_keyexpr_t *ke_out, - _z_subscription_infos_svec_t *infos) { - // Clear previous data - _z_subscription_cache_clear(&zn->_subscription_cache); - // Register new info - zn->_subscription_cache.ke_in = _z_keyexpr_duplicate(ke_in); - zn->_subscription_cache.ke_out = _z_keyexpr_duplicate(ke_out); - zn->_subscription_cache.infos = _z_subscription_infos_svec_alias(infos); - zn->_subscription_cache.sub_nb = _z_subscription_infos_svec_len(infos); -} - -void _z_subscription_cache_clear(_z_subscription_cache_t *cache) { - _z_subscription_infos_svec_clear(&cache->infos); - _z_keyexpr_clear(&cache->ke_in); - _z_keyexpr_clear(&cache->ke_out); -} - -#else -static inline bool _z_subscription_get_from_cache(_z_session_t *zn, const _z_keyexpr_t *ke, _z_keyexpr_t *ke_val, - _z_subscription_infos_svec_t *infos_val, size_t *sub_nb) { - _ZP_UNUSED(zn); - _ZP_UNUSED(ke); - _ZP_UNUSED(ke_val); - _ZP_UNUSED(infos_val); - _ZP_UNUSED(sub_nb); - return false; +static inline _z_subscription_cache_data_t _z_subscription_cache_data_null(void) { + _z_subscription_cache_data_t ret = {0}; + return ret; } -static inline void _z_subscription_update_cache(_z_session_t *zn, const _z_keyexpr_t *ke_in, const _z_keyexpr_t *ke_out, - _z_subscription_infos_svec_t *infos) { - _ZP_UNUSED(zn); - _ZP_UNUSED(ke_in); - _ZP_UNUSED(ke_out); - _ZP_UNUSED(infos); - return; +#if Z_FEATURE_RX_CACHE == 1 +int _z_subscription_cache_data_compare(const void *first, const void *second) { + _z_subscription_cache_data_t *first_data = (_z_subscription_cache_data_t *)first; + _z_subscription_cache_data_t *second_data = (_z_subscription_cache_data_t *)second; + return _z_keyexpr_compare(&first_data->ke_in, &second_data->ke_in); } #endif // Z_FEATURE_RX_CACHE == 1 -// Subscription bool _z_subscription_eq(const _z_subscription_t *other, const _z_subscription_t *this_) { return this_->_id == other->_id; } @@ -219,28 +181,45 @@ z_result_t _z_trigger_liveliness_subscriptions_undeclare(_z_session_t *zn, _z_ke Z_RELIABILITY_RELIABLE); } -static z_result_t _z_subscription_get_infos(_z_session_t *zn, _z_subscriber_kind_t kind, const _z_keyexpr_t *keyexpr, - _z_keyexpr_t *key, _z_subscription_infos_svec_t *subs, size_t *sub_nb) { +static z_result_t _z_subscription_get_infos(_z_session_t *zn, _z_subscriber_kind_t kind, + _z_subscription_cache_data_t *infos) { // Check cache - if (!_z_subscription_get_from_cache(zn, keyexpr, key, subs, sub_nb)) { - _Z_DEBUG("Resolving %d - %.*s on mapping 0x%x", keyexpr->_id, (int)_z_string_len(&keyexpr->_suffix), - _z_string_data(&keyexpr->_suffix), _z_keyexpr_mapping_id(keyexpr)); + _z_subscription_cache_data_t *cache_entry = NULL; +#if Z_FEATURE_RX_CACHE == 1 + cache_entry = _z_subscription_lru_cache_get(&zn->_subscription_cache, infos); +#endif + // Note cache entry + if (cache_entry != NULL) { + infos->ke_out = _z_keyexpr_alias(&cache_entry->ke_out); + infos->infos = _z_subscription_infos_svec_alias(&cache_entry->infos); + infos->sub_nb = cache_entry->sub_nb; + } else { // Construct data and add to cache + _Z_DEBUG("Resolving %d - %.*s on mapping 0x%x", infos->ke_in._id, (int)_z_string_len(&infos->ke_in._suffix), + _z_string_data(&infos->ke_in._suffix), _z_keyexpr_mapping_id(&infos->ke_in)); _z_session_mutex_lock(zn); - *key = __unsafe_z_get_expanded_key_from_key(zn, keyexpr, true); + infos->ke_out = __unsafe_z_get_expanded_key_from_key(zn, &infos->ke_in, true); - if (!_z_keyexpr_has_suffix(key)) { + if (!_z_keyexpr_has_suffix(&infos->ke_out)) { _z_session_mutex_unlock(zn); return _Z_ERR_KEYEXPR_UNKNOWN; } // Get subscription list - z_result_t ret = __unsafe_z_get_subscriptions_by_key(zn, kind, key, subs); + z_result_t ret = __unsafe_z_get_subscriptions_by_key(zn, kind, &infos->ke_out, &infos->infos); _z_session_mutex_unlock(zn); if (ret != _Z_RES_OK) { return ret; } - *sub_nb = _z_subscription_infos_svec_len(subs); - // Update cache - _z_subscription_update_cache(zn, keyexpr, key, subs); + infos->sub_nb = _z_subscription_infos_svec_len(&infos->infos); +#if Z_FEATURE_RX_CACHE == 1 + // Update cache, takes ownership of the data + _z_subscription_cache_data_t cache_storage = { + .infos = _z_subscription_infos_svec_alias(&infos->infos), + .ke_in = _z_keyexpr_duplicate(&infos->ke_in), + .ke_out = _z_keyexpr_duplicate(&infos->ke_out), + .sub_nb = infos->sub_nb, + }; + return _z_subscription_lru_cache_insert(&zn->_subscription_cache, &cache_storage); +#endif } return _Z_RES_OK; } @@ -250,35 +229,34 @@ static z_result_t _z_trigger_subscriptions_inner(_z_session_t *zn, _z_subscriber _z_encoding_t *encoding, const _z_zint_t sample_kind, const _z_timestamp_t *timestamp, const _z_n_qos_t qos, _z_bytes_t *attachment, z_reliability_t reliability) { - _z_keyexpr_t key; - _z_subscription_infos_svec_t subs; - size_t sub_nb; // Retrieve sub infos - _Z_RETURN_IF_ERR(_z_subscription_get_infos(zn, sub_kind, keyexpr, &key, &subs, &sub_nb)); + _z_subscription_cache_data_t sub_infos = _z_subscription_cache_data_null(); + sub_infos.ke_in = _z_keyexpr_alias(keyexpr); + _Z_RETURN_IF_ERR(_z_subscription_get_infos(zn, sub_kind, &sub_infos)); // Check if there are subs - _Z_DEBUG("Triggering %ju subs for key %d - %.*s", (uintmax_t)sub_nb, key._id, (int)_z_string_len(&key._suffix), - _z_string_data(&key._suffix)); - if (sub_nb == 0) { - _z_keyexpr_clear(&key); - _z_subscription_infos_svec_release(&subs); + _Z_DEBUG("Triggering %ju subs for key %d - %.*s", (uintmax_t)sub_infos.sub_nb, sub_infos.ke_out._id, + (int)_z_string_len(&sub_infos.ke_out._suffix), _z_string_data(&sub_infos.ke_out._suffix)); + if (sub_infos.sub_nb == 0) { + _z_keyexpr_clear(&sub_infos.ke_out); +#if Z_FEATURE_RX_CACHE == 0 + _z_subscription_infos_svec_release(&sub_infos.infos); // Otherwise it's released with cache +#endif return _Z_RES_OK; } // Create sample - _z_sample_t sample = _z_sample_alias(&key, payload, timestamp, encoding, sample_kind, qos, attachment, reliability); + _z_sample_t sample = + _z_sample_alias(&sub_infos.ke_out, payload, timestamp, encoding, sample_kind, qos, attachment, reliability); // Parse subscription infos svec - for (size_t i = 0; i < sub_nb; i++) { - _z_subscription_infos_t *sub_info = _z_subscription_infos_svec_get(&subs, i); + for (size_t i = 0; i < sub_infos.sub_nb; i++) { + _z_subscription_infos_t *sub_info = _z_subscription_infos_svec_get(&sub_infos.infos, i); sub_info->callback(&sample, sub_info->arg); } // Clean up - _z_keyexpr_clear(&key); - if (sub_kind == _Z_SUBSCRIBER_KIND_LIVELINESS_SUBSCRIBER) { - _z_subscription_infos_svec_release(&subs); - } else { -#if Z_FEATURE_RX_CACHE != 1 - _z_subscription_infos_svec_release(&subs); // Otherwise it's released with cache + _z_keyexpr_clear(&sub_infos.ke_out); +#if Z_FEATURE_RX_CACHE == 0 + _z_subscription_infos_svec_release(&sub_infos.infos); // Otherwise it's released with cache #endif - } + return _Z_RES_OK; } diff --git a/src/session/utils.c b/src/session/utils.c index 7bb9da967..dd349dd5c 100644 --- a/src/session/utils.c +++ b/src/session/utils.c @@ -78,7 +78,7 @@ z_result_t _z_session_init(_z_session_t *zn, const _z_id_t *zid) { zn->_subscriptions = NULL; zn->_liveliness_subscriptions = NULL; #if Z_FEATURE_RX_CACHE == 1 - memset(&zn->_subscription_cache, 0, sizeof(zn->_subscription_cache)); + zn->_subscription_cache = _z_subscription_lru_cache_init(Z_RX_CACHE_SIZE); #endif #endif #if Z_FEATURE_QUERYABLE == 1 @@ -126,7 +126,7 @@ void _z_session_clear(_z_session_t *zn) { #if Z_FEATURE_SUBSCRIPTION == 1 _z_flush_subscriptions(zn); #if Z_FEATURE_RX_CACHE == 1 - _z_subscription_cache_clear(&zn->_subscription_cache); + _z_subscription_lru_cache_delete(&zn->_subscription_cache); #endif #endif #if Z_FEATURE_QUERYABLE == 1 From 59215107e3c32b706caffdcda3752ffd1516f18a Mon Sep 17 00:00:00 2001 From: Jean-Roland Date: Thu, 19 Dec 2024 15:44:39 +0100 Subject: [PATCH 08/15] feat: use lru cache for queryable cache --- include/zenoh-pico/net/session.h | 2 +- include/zenoh-pico/session/queryable.h | 11 +- src/protocol/keyexpr.c | 17 ++- src/session/queryable.c | 158 ++++++++++++++----------- src/session/utils.c | 4 +- 5 files changed, 114 insertions(+), 78 deletions(-) diff --git a/include/zenoh-pico/net/session.h b/include/zenoh-pico/net/session.h index 463499cc9..5a608f933 100644 --- a/include/zenoh-pico/net/session.h +++ b/include/zenoh-pico/net/session.h @@ -86,7 +86,7 @@ typedef struct _z_session_t { #if Z_FEATURE_QUERYABLE == 1 _z_session_queryable_rc_list_t *_local_queryable; #if Z_FEATURE_RX_CACHE == 1 - _z_queryable_cache_t _queryable_cache; + _z_queryable_lru_cache_t _queryable_cache; #endif #endif #if Z_FEATURE_QUERY == 1 diff --git a/include/zenoh-pico/session/queryable.h b/include/zenoh-pico/session/queryable.h index 684880c17..c92527859 100644 --- a/include/zenoh-pico/session/queryable.h +++ b/include/zenoh-pico/session/queryable.h @@ -18,6 +18,8 @@ #include #include +#include "zenoh-pico/collections/lru_cache.h" + // Forward declaration to avoid cyclical include typedef struct _z_session_t _z_session_t; typedef struct _z_session_rc_t _z_session_rc_t; @@ -36,14 +38,19 @@ typedef struct { _z_keyexpr_t ke_out; _z_queryable_infos_svec_t infos; size_t qle_nb; -} _z_queryable_cache_t; +} _z_queryable_cache_data_t; + +int _z_queryable_cache_data_compare(const void *first, const void *second); #if Z_FEATURE_QUERYABLE == 1 #define _Z_QUERYABLE_COMPLETE_DEFAULT false #define _Z_QUERYABLE_DISTANCE_DEFAULT 0 +#if Z_FEATURE_RX_CACHE == 1 +_Z_LRU_CACHE_DEFINE(_z_queryable, _z_queryable_cache_data_t, _z_queryable_cache_data_compare) +#endif + /*------------------ Queryable ------------------*/ -void _z_queryable_cache_clear(_z_queryable_cache_t *cache); _z_session_queryable_rc_t *_z_get_session_queryable_by_id(_z_session_t *zn, const _z_zint_t id); _z_session_queryable_rc_t *_z_register_session_queryable(_z_session_t *zn, _z_session_queryable_t *q); z_result_t _z_trigger_queryables(_z_session_rc_t *zn, _z_msg_query_t *query, _z_keyexpr_t *q_key, uint32_t qid); diff --git a/src/protocol/keyexpr.c b/src/protocol/keyexpr.c index 35d8a5f5c..1eb93326a 100644 --- a/src/protocol/keyexpr.c +++ b/src/protocol/keyexpr.c @@ -35,12 +35,19 @@ _z_keyexpr_t _z_rid_with_suffix(uint16_t rid, const char *suffix) { int _z_keyexpr_compare(_z_keyexpr_t *first, _z_keyexpr_t *second) { if ((first->_id != 0) && (second->_id != 0)) { - if (first->_id == second->_id) { - return 0; - } else if (first->_id > second->_id) { - return 1; + if (_z_keyexpr_mapping_id(first) == _z_keyexpr_mapping_id(second)) { + if (first->_id == second->_id) { + return 0; + } else if (first->_id > second->_id) { + return 1; + } + return -1; + } else { + if (_z_keyexpr_mapping_id(first) > _z_keyexpr_mapping_id(second)) { + return 1; + } + return -1; } - return -1; } if (_z_keyexpr_has_suffix(first) && _z_keyexpr_has_suffix(second)) { return _z_string_compare(&first->_suffix, &second->_suffix); diff --git a/src/session/queryable.c b/src/session/queryable.c index 8b84a03f3..19be0693b 100644 --- a/src/session/queryable.c +++ b/src/session/queryable.c @@ -32,56 +32,62 @@ #define _Z_QLEINFOS_VEC_SIZE 4 // Arbitrary initial size -#if Z_FEATURE_RX_CACHE == 1 -static inline bool _z_queryable_get_from_cache(_z_session_t *zn, const _z_keyexpr_t *ke, _z_keyexpr_t *ke_val, - _z_queryable_infos_svec_t *infos_val, size_t *qle_nb) { - if (!_z_keyexpr_equals(ke, &zn->_queryable_cache.ke_in)) { - return false; - } - *ke_val = _z_keyexpr_alias(&zn->_queryable_cache.ke_out); - *infos_val = _z_queryable_infos_svec_alias(&zn->_queryable_cache.infos); - *qle_nb = zn->_queryable_cache.qle_nb; - return true; -} - -static inline void _z_queryable_update_cache(_z_session_t *zn, const _z_keyexpr_t *ke_in, const _z_keyexpr_t *ke_out, - _z_queryable_infos_svec_t *infos) { - // Clear previous data - _z_queryable_cache_clear(&zn->_queryable_cache); - // Register new info - zn->_queryable_cache.ke_in = _z_keyexpr_duplicate(ke_in); - zn->_queryable_cache.ke_out = _z_keyexpr_duplicate(ke_out); - zn->_queryable_cache.infos = _z_queryable_infos_svec_alias(infos); - zn->_queryable_cache.qle_nb = _z_queryable_infos_svec_len(infos); -} - -void _z_queryable_cache_clear(_z_queryable_cache_t *cache) { - _z_queryable_infos_svec_clear(&cache->infos); - _z_keyexpr_clear(&cache->ke_in); - _z_keyexpr_clear(&cache->ke_out); -} - -#else -static inline bool _z_queryable_get_from_cache(_z_session_t *zn, const _z_keyexpr_t *ke, _z_keyexpr_t *ke_val, - _z_queryable_infos_svec_t *infos_val, size_t *sub_nb) { - _ZP_UNUSED(zn); - _ZP_UNUSED(ke); - _ZP_UNUSED(ke_val); - _ZP_UNUSED(infos_val); - _ZP_UNUSED(sub_nb); - return false; +static inline _z_queryable_cache_data_t _z_queryable_cache_data_null(void) { + _z_queryable_cache_data_t ret = {0}; + return ret; } -static inline void _z_queryable_update_cache(_z_session_t *zn, const _z_keyexpr_t *ke_in, const _z_keyexpr_t *ke_out, - _z_queryable_infos_svec_t *infos) { - _ZP_UNUSED(zn); - _ZP_UNUSED(ke_in); - _ZP_UNUSED(ke_out); - _ZP_UNUSED(infos); - return; +#if Z_FEATURE_RX_CACHE == 1 +int _z_queryable_cache_data_compare(const void *first, const void *second) { + _z_queryable_cache_data_t *first_data = (_z_queryable_cache_data_t *)first; + _z_queryable_cache_data_t *second_data = (_z_queryable_cache_data_t *)second; + return _z_keyexpr_compare(&first_data->ke_in, &second_data->ke_in); } #endif // Z_FEATURE_RX_CACHE == 1 +// #if Z_FEATURE_RX_CACHE == 1 +// static inline bool _z_queryable_get_from_cache(_z_session_t *zn, const _z_keyexpr_t *ke, _z_keyexpr_t *ke_val, +// _z_queryable_infos_svec_t *infos_val, size_t *qle_nb) { +// if (!_z_keyexpr_equals(ke, &zn->_queryable_cache.ke_in)) { +// return false; +// } +// *ke_val = _z_keyexpr_alias(&zn->_queryable_cache.ke_out); +// *infos_val = _z_queryable_infos_svec_alias(&zn->_queryable_cache.infos); +// *qle_nb = zn->_queryable_cache.qle_nb; +// return true; +// } + +// static inline void _z_queryable_update_cache(_z_session_t *zn, const _z_keyexpr_t *ke_in, const _z_keyexpr_t *ke_out, +// _z_queryable_infos_svec_t *infos) { +// // Clear previous data +// _z_queryable_cache_clear(&zn->_queryable_cache); +// // Register new info +// zn->_queryable_cache.ke_in = _z_keyexpr_duplicate(ke_in); +// zn->_queryable_cache.ke_out = _z_keyexpr_duplicate(ke_out); +// zn->_queryable_cache.infos = _z_queryable_infos_svec_alias(infos); +// zn->_queryable_cache.qle_nb = _z_queryable_infos_svec_len(infos); +// } +// #else +// static inline bool _z_queryable_get_from_cache(_z_session_t *zn, const _z_keyexpr_t *ke, _z_keyexpr_t *ke_val, +// _z_queryable_infos_svec_t *infos_val, size_t *sub_nb) { +// _ZP_UNUSED(zn); +// _ZP_UNUSED(ke); +// _ZP_UNUSED(ke_val); +// _ZP_UNUSED(infos_val); +// _ZP_UNUSED(sub_nb); +// return false; +// } + +// static inline void _z_queryable_update_cache(_z_session_t *zn, const _z_keyexpr_t *ke_in, const _z_keyexpr_t *ke_out, +// _z_queryable_infos_svec_t *infos) { +// _ZP_UNUSED(zn); +// _ZP_UNUSED(ke_in); +// _ZP_UNUSED(ke_out); +// _ZP_UNUSED(infos); +// return; +// } +// #endif // Z_FEATURE_RX_CACHE == 1 + bool _z_session_queryable_eq(const _z_session_queryable_t *one, const _z_session_queryable_t *two) { return one->_id == two->_id; } @@ -174,28 +180,45 @@ _z_session_queryable_rc_t *_z_register_session_queryable(_z_session_t *zn, _z_se return ret; } -static z_result_t _z_session_queryable_get_infos(_z_session_t *zn, const _z_keyexpr_t *keyexpr, _z_keyexpr_t *key, - _z_queryable_infos_svec_t *qles, size_t *qle_nb) { +static z_result_t _z_session_queryable_get_infos(_z_session_t *zn, _z_queryable_cache_data_t *infos) { + _z_queryable_cache_data_t *cache_entry = NULL; +#if Z_FEATURE_RX_CACHE == 1 + cache_entry = _z_queryable_lru_cache_get(&zn->_queryable_cache, infos); +#endif // Check cache - if (!_z_queryable_get_from_cache(zn, keyexpr, key, qles, qle_nb)) { - _Z_DEBUG("Resolving %d - %.*s on mapping 0x%x", keyexpr->_id, (int)_z_string_len(&keyexpr->_suffix), - _z_string_data(&keyexpr->_suffix), _z_keyexpr_mapping_id(keyexpr)); + if (cache_entry != NULL) { + // Note cache entry + infos->ke_out = _z_keyexpr_alias(&cache_entry->ke_out); + infos->infos = _z_queryable_infos_svec_alias(&cache_entry->infos); + infos->qle_nb = cache_entry->qle_nb; + } else { + // Build queryable data + _Z_DEBUG("Resolving %d - %.*s on mapping 0x%x", infos->ke_in._id, (int)_z_string_len(&infos->ke_in._suffix), + _z_string_data(&infos->ke_in._suffix), _z_keyexpr_mapping_id(&infos->ke_in)); _z_session_mutex_lock(zn); - *key = __unsafe_z_get_expanded_key_from_key(zn, keyexpr, true); + infos->ke_out = __unsafe_z_get_expanded_key_from_key(zn, &infos->ke_in, true); - if (!_z_keyexpr_has_suffix(key)) { + if (!_z_keyexpr_has_suffix(&infos->ke_out)) { _z_session_mutex_unlock(zn); return _Z_ERR_KEYEXPR_UNKNOWN; } // Get queryable list - z_result_t ret = __unsafe_z_get_session_queryable_by_key(zn, key, qles); + z_result_t ret = __unsafe_z_get_session_queryable_by_key(zn, &infos->ke_out, &infos->infos); _z_session_mutex_unlock(zn); if (ret != _Z_RES_OK) { return ret; } - *qle_nb = _z_queryable_infos_svec_len(qles); + infos->qle_nb = _z_queryable_infos_svec_len(&infos->infos); +#if Z_FEATURE_RX_CACHE == 1 // Update cache - _z_queryable_update_cache(zn, keyexpr, key, qles); + _z_queryable_cache_data_t cache_storage = { + .infos = _z_queryable_infos_svec_alias(&infos->infos), + .ke_in = _z_keyexpr_duplicate(&infos->ke_in), + .ke_out = _z_keyexpr_duplicate(&infos->ke_out), + .qle_nb = infos->qle_nb, + }; + _z_queryable_lru_cache_insert(&zn->_queryable_cache, &cache_storage); +#endif } return _Z_RES_OK; } @@ -212,16 +235,15 @@ static inline void _z_queryable_query_steal_data(_z_query_t *query, _z_session_r static z_result_t _z_trigger_queryables_inner(_z_session_rc_t *zsrc, _z_msg_query_t *msgq, _z_keyexpr_t *q_key, uint32_t qid) { _z_session_t *zn = _Z_RC_IN_VAL(zsrc); - _z_keyexpr_t key; - _z_queryable_infos_svec_t qles; - size_t qle_nb; + _z_queryable_cache_data_t qle_infos = _z_queryable_cache_data_null(); + qle_infos.ke_in = _z_keyexpr_alias(q_key); // Retrieve sub infos - _Z_RETURN_IF_ERR(_z_session_queryable_get_infos(zn, q_key, &key, &qles, &qle_nb)); + _Z_RETURN_IF_ERR(_z_session_queryable_get_infos(zn, &qle_infos)); // Check if there are queryables - _Z_DEBUG("Triggering %ju queryables for key %d - %.*s", (uintmax_t)qle_nb, key._id, - (int)_z_string_len(&key._suffix), _z_string_data(&key._suffix)); - if (qle_nb == 0) { - _z_keyexpr_clear(&key); + _Z_DEBUG("Triggering %ju queryables for key %d - %.*s", (uintmax_t)qle_infos.qle_nb, qle_infos.ke_out._id, + (int)_z_string_len(&qle_infos.ke_out._suffix), _z_string_data(&qle_infos.ke_out._suffix)); + if (qle_infos.qle_nb == 0) { + _z_keyexpr_clear(&qle_infos.ke_out); return _Z_RES_OK; } // Check anyke @@ -241,17 +263,17 @@ static z_result_t _z_trigger_queryables_inner(_z_session_rc_t *zsrc, _z_msg_quer if (_Z_RC_IS_NULL(&query)) { return _Z_ERR_SYSTEM_OUT_OF_MEMORY; } - _z_queryable_query_steal_data(q, zsrc, msgq, &key, qid, anyke); + _z_queryable_query_steal_data(q, zsrc, msgq, &qle_infos.ke_out, qid, anyke); // Parse session_queryable svec - for (size_t i = 0; i < qle_nb; i++) { - _z_queryable_infos_t *qle_info = _z_queryable_infos_svec_get(&qles, i); + for (size_t i = 0; i < qle_infos.qle_nb; i++) { + _z_queryable_infos_t *qle_info = _z_queryable_infos_svec_get(&qle_infos.infos, i); qle_info->callback(&query, qle_info->arg); } _z_query_rc_drop(&query); // Clean up - _z_keyexpr_clear(&key); + _z_keyexpr_clear(&qle_infos.ke_out); #if Z_FEATURE_RX_CACHE != 1 - _z_queryable_infos_svec_release(&qles); // Otherwise it's released with cache + _z_queryable_infos_svec_release(&qle_infos.infos); // Otherwise it's released with cache #endif return _Z_RES_OK; } diff --git a/src/session/utils.c b/src/session/utils.c index dd349dd5c..2a9c3f7d5 100644 --- a/src/session/utils.c +++ b/src/session/utils.c @@ -84,7 +84,7 @@ z_result_t _z_session_init(_z_session_t *zn, const _z_id_t *zid) { #if Z_FEATURE_QUERYABLE == 1 zn->_local_queryable = NULL; #if Z_FEATURE_RX_CACHE == 1 - memset(&zn->_queryable_cache, 0, sizeof(zn->_queryable_cache)); + zn->_queryable_cache = _z_queryable_lru_cache_init(Z_RX_CACHE_SIZE); #endif #endif #if Z_FEATURE_QUERY == 1 @@ -132,7 +132,7 @@ void _z_session_clear(_z_session_t *zn) { #if Z_FEATURE_QUERYABLE == 1 _z_flush_session_queryable(zn); #if Z_FEATURE_RX_CACHE == 1 - _z_queryable_cache_clear(&zn->_queryable_cache); + _z_queryable_lru_cache_delete(&zn->_queryable_cache); #endif #endif #if Z_FEATURE_QUERY == 1 From 7d35bdc45b04ba8c4de893a355ab2f4bc6d73753 Mon Sep 17 00:00:00 2001 From: Jean-Roland Date: Thu, 19 Dec 2024 16:43:52 +0100 Subject: [PATCH 09/15] feat: add lru bench and tree config token --- include/zenoh-pico/collections/lru_cache.h | 3 -- include/zenoh-pico/config.h | 9 +++++ include/zenoh-pico/config.h.in | 9 +++++ tests/z_lru_cache_test.c | 40 ++++++++++++++++++++++ 4 files changed, 58 insertions(+), 3 deletions(-) diff --git a/include/zenoh-pico/collections/lru_cache.h b/include/zenoh-pico/collections/lru_cache.h index 0f935d642..f60eca4c1 100644 --- a/include/zenoh-pico/collections/lru_cache.h +++ b/include/zenoh-pico/collections/lru_cache.h @@ -23,9 +23,6 @@ extern "C" { #endif -// TODO: move to config -#define Z_FEATURE_CACHE_TREE 0 - // Three way comparison function pointer typedef int (*_z_lru_val_cmp_f)(const void *first, const void *second); diff --git a/include/zenoh-pico/config.h b/include/zenoh-pico/config.h index 3542c7d1d..d5883e121 100644 --- a/include/zenoh-pico/config.h +++ b/include/zenoh-pico/config.h @@ -180,6 +180,15 @@ */ #define Z_RX_CACHE_SIZE 10 +/** + * Use binary tree to speed up rx cache search + */ +#if Z_RX_CACHE_SIZE > 20 +#define Z_FEATURE_CACHE_TREE 1 +#else +#define Z_FEATURE_CACHE_TREE 0 +#endif + /** * Default get timeout in milliseconds. */ diff --git a/include/zenoh-pico/config.h.in b/include/zenoh-pico/config.h.in index a3748c432..2586d7e9e 100644 --- a/include/zenoh-pico/config.h.in +++ b/include/zenoh-pico/config.h.in @@ -180,6 +180,15 @@ */ #define Z_RX_CACHE_SIZE 10 +/** + * Use binary tree to speed up rx cache search + */ +#if Z_RX_CACHE_SIZE > 20 +#define Z_FEATURE_CACHE_TREE 1 +#else +#define Z_FEATURE_CACHE_TREE 0 +#endif + /** * Default get timeout in milliseconds. */ diff --git a/tests/z_lru_cache_test.c b/tests/z_lru_cache_test.c index b556d4584..64dc4e7e8 100644 --- a/tests/z_lru_cache_test.c +++ b/tests/z_lru_cache_test.c @@ -184,11 +184,51 @@ void test_lru_cache_random_val(void) { _dummy_lru_cache_delete(&dcache); } +#if 0 +void *stop_task(void *ctx) { + z_sleep_s(10); + bool *stop_flag = (bool *)ctx; + *stop_flag = true; + return NULL; +} +// Results +// size 10: 287720681 list, 294626805 tree (1.024) +// size 20: 207124561 list, 282312820 tree (1.363) +// size 50: 104164238 list, 212367113 tree (2.039) +// size 100: 51666425 list, 198541036 tree (3.843) +// size 1000: 5838653 list, 122717170 tree (21.018) +void test_search_benchmark(void) { + _dummy_lru_cache_t dcache = _dummy_lru_cache_init(10); + _dummy_t data[10] = {0}; + + srand(0x55); + // Insert data + for (size_t i = 0; i < _ZP_ARRAY_SIZE(data); i++) { + data[i].foo = rand(); + assert(_dummy_lru_cache_insert(&dcache, &data[i]) == 0); + } + bool stop_flag = false; + pthread_t task; + pthread_create(&task, NULL, stop_task, &stop_flag); + size_t get_cnt = 0; + + while (!stop_flag) { + int i = rand() % _ZP_ARRAY_SIZE(data); + _dummy_t *src = &data[i]; + _dummy_t *res = _dummy_lru_cache_get(&dcache, src); + get_cnt++; + assert(res != NULL); + } + printf("Mode: %d, Get count: %ld\n", Z_FEATURE_CACHE_TREE, get_cnt); +} +#endif + int main(void) { test_lru_init(); test_lru_cache_insert(); test_lru_cache_deletion(); test_lru_cache_update(); test_lru_cache_random_val(); + // test_search_benchmark(); return 0; } From 3fbb434bc9cf0b4ff248bcbf2ddbe13b427aa955 Mon Sep 17 00:00:00 2001 From: Jean-Roland Gosse Date: Thu, 19 Dec 2024 21:29:16 +0100 Subject: [PATCH 10/15] fix: improve _z_string_compare Co-authored-by: Alexander Bushnev --- src/collections/string.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/collections/string.c b/src/collections/string.c index 7cc72ce1f..e3ce9f422 100644 --- a/src/collections/string.c +++ b/src/collections/string.c @@ -102,10 +102,20 @@ void _z_string_free(_z_string_t **str) { } int _z_string_compare(const _z_string_t *left, const _z_string_t *right) { - if (_z_string_len(left) <= _z_string_len(right)) { - return strncmp(_z_string_data(left), _z_string_data(right), _z_string_len(left)); + size_t len_left = _z_string_len(left); + size_t len_right = _z_string_len(right); + + int result = strncmp(_z_string_data(left), _z_string_data(right), len_left < len_right ? len_left : len_right); + + if (result == 0) { + if (len_left < len_right) { + return -1; + } else if (len_left > len_right) { + return 1; + } } - return strncmp(_z_string_data(left), _z_string_data(right), _z_string_len(right)); + + return result; } bool _z_string_equals(const _z_string_t *left, const _z_string_t *right) { From 210f92aa0202332926412afccc8f8783bfead05e Mon Sep 17 00:00:00 2001 From: Jean-Roland Date: Thu, 19 Dec 2024 21:30:23 +0100 Subject: [PATCH 11/15] fix: remove obsolete code --- src/session/queryable.c | 43 ----------------------------------------- 1 file changed, 43 deletions(-) diff --git a/src/session/queryable.c b/src/session/queryable.c index 19be0693b..a4c7106a0 100644 --- a/src/session/queryable.c +++ b/src/session/queryable.c @@ -45,49 +45,6 @@ int _z_queryable_cache_data_compare(const void *first, const void *second) { } #endif // Z_FEATURE_RX_CACHE == 1 -// #if Z_FEATURE_RX_CACHE == 1 -// static inline bool _z_queryable_get_from_cache(_z_session_t *zn, const _z_keyexpr_t *ke, _z_keyexpr_t *ke_val, -// _z_queryable_infos_svec_t *infos_val, size_t *qle_nb) { -// if (!_z_keyexpr_equals(ke, &zn->_queryable_cache.ke_in)) { -// return false; -// } -// *ke_val = _z_keyexpr_alias(&zn->_queryable_cache.ke_out); -// *infos_val = _z_queryable_infos_svec_alias(&zn->_queryable_cache.infos); -// *qle_nb = zn->_queryable_cache.qle_nb; -// return true; -// } - -// static inline void _z_queryable_update_cache(_z_session_t *zn, const _z_keyexpr_t *ke_in, const _z_keyexpr_t *ke_out, -// _z_queryable_infos_svec_t *infos) { -// // Clear previous data -// _z_queryable_cache_clear(&zn->_queryable_cache); -// // Register new info -// zn->_queryable_cache.ke_in = _z_keyexpr_duplicate(ke_in); -// zn->_queryable_cache.ke_out = _z_keyexpr_duplicate(ke_out); -// zn->_queryable_cache.infos = _z_queryable_infos_svec_alias(infos); -// zn->_queryable_cache.qle_nb = _z_queryable_infos_svec_len(infos); -// } -// #else -// static inline bool _z_queryable_get_from_cache(_z_session_t *zn, const _z_keyexpr_t *ke, _z_keyexpr_t *ke_val, -// _z_queryable_infos_svec_t *infos_val, size_t *sub_nb) { -// _ZP_UNUSED(zn); -// _ZP_UNUSED(ke); -// _ZP_UNUSED(ke_val); -// _ZP_UNUSED(infos_val); -// _ZP_UNUSED(sub_nb); -// return false; -// } - -// static inline void _z_queryable_update_cache(_z_session_t *zn, const _z_keyexpr_t *ke_in, const _z_keyexpr_t *ke_out, -// _z_queryable_infos_svec_t *infos) { -// _ZP_UNUSED(zn); -// _ZP_UNUSED(ke_in); -// _ZP_UNUSED(ke_out); -// _ZP_UNUSED(infos); -// return; -// } -// #endif // Z_FEATURE_RX_CACHE == 1 - bool _z_session_queryable_eq(const _z_session_queryable_t *one, const _z_session_queryable_t *two) { return one->_id == two->_id; } From 9c056f44f8114f45e151f9538486d80d629d50ea Mon Sep 17 00:00:00 2001 From: Jean-Roland Date: Fri, 20 Dec 2024 10:18:00 +0100 Subject: [PATCH 12/15] feat: switch tree to sorted array --- include/zenoh-pico/collections/lru_cache.h | 15 +- include/zenoh-pico/config.h | 9 - include/zenoh-pico/config.h.in | 9 - src/collections/lru_cache.c | 219 ++++++--------------- tests/z_lru_cache_test.c | 9 +- 5 files changed, 74 insertions(+), 187 deletions(-) diff --git a/include/zenoh-pico/collections/lru_cache.h b/include/zenoh-pico/collections/lru_cache.h index f60eca4c1..1bed89b27 100644 --- a/include/zenoh-pico/collections/lru_cache.h +++ b/include/zenoh-pico/collections/lru_cache.h @@ -15,9 +15,10 @@ #ifndef ZENOH_PICO_COLLECTIONS_LRUCACHE_H #define ZENOH_PICO_COLLECTIONS_LRUCACHE_H +#include #include -#include "zenoh-pico/collections/element.h" +#include "zenoh-pico/utils/result.h" #ifdef __cplusplus extern "C" { @@ -34,13 +35,11 @@ typedef void _z_lru_cache_node_t; * A least recently used cache implementation */ typedef struct _z_lru_cache_t { - size_t capacity; // Max number of node - size_t len; // Number of node - _z_lru_cache_node_t *head; // List head - _z_lru_cache_node_t *tail; // List tail -#if Z_FEATURE_CACHE_TREE == 1 - _z_lru_cache_node_t *root; // Tree root -#endif + size_t capacity; // Max number of node + size_t len; // Number of node + _z_lru_cache_node_t *head; // List head + _z_lru_cache_node_t *tail; // List tail + _z_lru_cache_node_t **slist; // Sorted node list } _z_lru_cache_t; _z_lru_cache_t _z_lru_cache_init(size_t capacity); diff --git a/include/zenoh-pico/config.h b/include/zenoh-pico/config.h index d5883e121..3542c7d1d 100644 --- a/include/zenoh-pico/config.h +++ b/include/zenoh-pico/config.h @@ -180,15 +180,6 @@ */ #define Z_RX_CACHE_SIZE 10 -/** - * Use binary tree to speed up rx cache search - */ -#if Z_RX_CACHE_SIZE > 20 -#define Z_FEATURE_CACHE_TREE 1 -#else -#define Z_FEATURE_CACHE_TREE 0 -#endif - /** * Default get timeout in milliseconds. */ diff --git a/include/zenoh-pico/config.h.in b/include/zenoh-pico/config.h.in index 2586d7e9e..a3748c432 100644 --- a/include/zenoh-pico/config.h.in +++ b/include/zenoh-pico/config.h.in @@ -180,15 +180,6 @@ */ #define Z_RX_CACHE_SIZE 10 -/** - * Use binary tree to speed up rx cache search - */ -#if Z_RX_CACHE_SIZE > 20 -#define Z_FEATURE_CACHE_TREE 1 -#else -#define Z_FEATURE_CACHE_TREE 0 -#endif - /** * Default get timeout in milliseconds. */ diff --git a/src/collections/lru_cache.c b/src/collections/lru_cache.c index 81505755e..aa0f62bd9 100644 --- a/src/collections/lru_cache.c +++ b/src/collections/lru_cache.c @@ -23,14 +23,10 @@ #include "zenoh-pico/utils/pointers.h" #include "zenoh-pico/utils/result.h" -// Nodes are chained both as a binary tree for lookup and as double linked list for lru insertion/deletion. +// Nodes are chained as double linked list for lru insertion/deletion. typedef struct _z_lru_cache_node_data_t { _z_lru_cache_node_t *prev; // List previous node _z_lru_cache_node_t *next; // List next node -#if Z_FEATURE_CACHE_TREE == 1 - _z_lru_cache_node_t *left; // Tree left child - _z_lru_cache_node_t *right; // Tree right child -#endif } _z_lru_cache_node_data_t; #define NODE_DATA_SIZE sizeof(_z_lru_cache_node_data_t) @@ -106,176 +102,79 @@ static void _z_lru_cache_update_list(_z_lru_cache_t *cache, _z_lru_cache_node_t _z_lru_cache_insert_list_node(cache, node); } -// Tree functions -#if Z_FEATURE_CACHE_TREE == 1 -static _z_lru_cache_node_t *_z_lru_cache_insert_tree_node_rec(_z_lru_cache_node_t *previous, _z_lru_cache_node_t *node, - _z_lru_val_cmp_f compare) { - if (previous == NULL) { - return node; - } - _z_lru_cache_node_data_t *prev_data = _z_lru_cache_node_data(previous); - int res = compare(_z_lru_cache_node_value(previous), _z_lru_cache_node_value(node)); - if (res < 0) { - prev_data->left = _z_lru_cache_insert_tree_node_rec(prev_data->left, node, compare); - } else if (res > 0) { - prev_data->right = _z_lru_cache_insert_tree_node_rec(prev_data->right, node, compare); - } - // == 0 shouldn't happen - else { - _Z_ERROR("Comparison equality shouldn't happen during tree insertion"); - } - return previous; -} - -static void _z_lru_cache_insert_tree_node(_z_lru_cache_t *cache, _z_lru_cache_node_t *node, _z_lru_val_cmp_f compare) { - cache->root = _z_lru_cache_insert_tree_node_rec(cache->root, node, compare); -} - -static _z_lru_cache_node_t *_z_lru_cache_search_tree_rec(_z_lru_cache_node_t *node, void *value, - _z_lru_val_cmp_f compare) { - // Check value - if (node == NULL) { - return NULL; - } - // Compare keys - int res = compare(_z_lru_cache_node_value(node), value); - // Node has key - if (res == 0) { - return node; - } - _z_lru_cache_node_data_t *node_data = _z_lru_cache_node_data(node); - // Key is greater than node's key - if (res > 0) { - return _z_lru_cache_search_tree_rec(node_data->right, value, compare); - } - // Key is smaller than node's key - return _z_lru_cache_search_tree_rec(node_data->left, value, compare); -} - -static _z_lru_cache_node_t *_z_lru_cache_search_tree(_z_lru_cache_t *cache, void *value, _z_lru_val_cmp_f compare) { - return _z_lru_cache_search_tree_rec(cache->root, value, compare); -} - -static _z_lru_cache_node_t *_z_lru_cache_get_tree_node_successor(_z_lru_cache_node_t *node) { - _z_lru_cache_node_data_t *node_data = _z_lru_cache_node_data(node); - node = node_data->right; - node_data = _z_lru_cache_node_data(node); - while ((node != NULL) && (node_data->left != NULL)) { - node = node_data->left; - node_data = _z_lru_cache_node_data(node); +// Sorted list function +static _z_lru_cache_node_t *_z_lru_cache_search_slist(_z_lru_cache_t *cache, void *value, _z_lru_val_cmp_f compare, + size_t *idx) { + ssize_t l_idx = 0; + ssize_t h_idx = (ssize_t)cache->len - 1; + while (l_idx <= h_idx) { + ssize_t curr_idx = (l_idx + h_idx) / 2; + int res = compare(_z_lru_cache_node_value(cache->slist[curr_idx]), value); + if (res == 0) { + *idx = (size_t)curr_idx; + return cache->slist[curr_idx]; + } else if (res < 0) { + l_idx = curr_idx + 1; + } else { + h_idx = curr_idx - 1; + } } - return node; + return NULL; } -static void _z_lru_cache_transfer_tree_node(_z_lru_cache_node_t *dst, _z_lru_cache_node_t *src, size_t value_size) { - void *src_val = _z_lru_cache_node_value(src); - void *dst_val = _z_lru_cache_node_value(dst); - _z_lru_cache_node_data_t *src_data = _z_lru_cache_node_data(src); - _z_lru_cache_node_data_t *dst_data = _z_lru_cache_node_data(dst); - // Copy content - memcpy(dst_val, src_val, value_size); - dst_data->prev = src_data->prev; - dst_data->next = src_data->next; - // Update list - if (src_data->prev != NULL) { - _z_lru_cache_node_data_t *prev_data = _z_lru_cache_node_data(src_data->prev); - prev_data->next = dst; - } - if (src_data->next != NULL) { - _z_lru_cache_node_data_t *next_data = _z_lru_cache_node_data(src_data->next); - next_data->prev = dst; +static void _z_lru_cache_sort_slist(_z_lru_cache_node_t **slist, size_t slist_size, _z_lru_val_cmp_f compare) { + for (size_t i = 1; i < slist_size; i++) { + _z_lru_cache_node_t *node = slist[i]; + void *node_val = _z_lru_cache_node_value(node); + ssize_t j = (ssize_t)i - 1; + while ((j >= 0) && (compare(_z_lru_cache_node_value(slist[j]), node_val) > 0)) { + slist[j + 1] = slist[j]; + j--; + } + slist[j + 1] = node; } } -static _z_lru_cache_node_t *_z_lru_cache_delete_tree_node_rec(_z_lru_cache_node_t *node, void *value, size_t value_size, - _z_lru_val_cmp_f compare) { - // No node - if (node == NULL) { - return NULL; - } - // Compare value - _z_lru_cache_node_data_t *node_data = _z_lru_cache_node_data(node); - void *node_val = _z_lru_cache_node_value(node); - int res = compare(node_val, value); - if (res < 0) { - node_data->left = _z_lru_cache_delete_tree_node_rec(node_data->left, value, value_size, compare); - } else if (res > 0) { - node_data->right = _z_lru_cache_delete_tree_node_rec(node_data->right, value, value_size, compare); - } else { // Node found - // No child / one child - if (node_data->left == NULL) { - _z_lru_cache_node_t *temp = node_data->right; - z_free(node); - return temp; - } else if (node_data->right == NULL) { - _z_lru_cache_node_t *temp = node_data->left; - z_free(node); - return temp; - } - // Two children - _z_lru_cache_node_t *succ = _z_lru_cache_get_tree_node_successor(node); - // Transfer successor data to node - _z_lru_cache_transfer_tree_node(node, succ, value_size); - node_data->right = - _z_lru_cache_delete_tree_node_rec(node_data->right, _z_lru_cache_node_value(succ), value_size, compare); +static void _z_lru_cache_insert_slist(_z_lru_cache_t *cache, _z_lru_cache_node_t *node, _z_lru_val_cmp_f compare, + size_t *idx) { + if (idx != NULL) { + assert(*idx < cache->capacity); + cache->slist[*idx] = node; + } else { + assert(cache->len < cache->capacity); + assert(cache->slist[cache->len] == NULL); + cache->slist[cache->len] = node; } - return node; + _z_lru_cache_sort_slist(cache->slist, cache->len + 1, compare); } -static void _z_lru_cache_delete_tree_node(_z_lru_cache_t *cache, _z_lru_cache_node_t *node, size_t value_size, - _z_lru_val_cmp_f compare) { - cache->root = _z_lru_cache_delete_tree_node_rec(cache->root, _z_lru_cache_node_value(node), value_size, compare); +static size_t _z_lru_cache_delete_slist(_z_lru_cache_t *cache, _z_lru_cache_node_t *node, _z_lru_val_cmp_f compare) { + size_t idx = 0; + (void)_z_lru_cache_search_slist(cache, _z_lru_cache_node_value(node), compare, &idx); + return idx; } -#else -_z_lru_cache_node_t *_z_lru_cache_search_list(_z_lru_cache_t *cache, void *value, _z_lru_val_cmp_f compare) { - if (cache->head == NULL) { - return NULL; - } - _z_lru_cache_node_data_t *node = cache->head; - // Parse list - while (node != NULL) { - _z_lru_cache_node_data_t *node_data = _z_lru_cache_node_data(node); - void *node_val = _z_lru_cache_node_value(node); - if (compare(node_val, value) == 0) { - return node; - } - node = node_data->next; - } - return NULL; -} -#endif // Main static functions -static void _z_lru_cache_delete_last(_z_lru_cache_t *cache, size_t value_size, _z_lru_val_cmp_f compare) { +static size_t _z_lru_cache_delete_last(_z_lru_cache_t *cache, _z_lru_val_cmp_f compare) { _z_lru_cache_node_t *last = cache->tail; assert(last != NULL); _z_lru_cache_remove_list_node(cache, last); -#if Z_FEATURE_CACHE_TREE == 1 - _z_lru_cache_delete_tree_node(cache, last, value_size, compare); -#else - _ZP_UNUSED(compare); - _ZP_UNUSED(value_size); + size_t idx = _z_lru_cache_delete_slist(cache, last, compare); z_free(last); -#endif cache->len--; + return idx; } -static void _z_lru_cache_insert_node(_z_lru_cache_t *cache, _z_lru_cache_node_t *node, _z_lru_val_cmp_f compare) { +static void _z_lru_cache_insert_node(_z_lru_cache_t *cache, _z_lru_cache_node_t *node, _z_lru_val_cmp_f compare, + size_t *idx) { _z_lru_cache_insert_list_node(cache, node); -#if Z_FEATURE_CACHE_TREE == 1 - _z_lru_cache_insert_tree_node(cache, node, compare); -#else - _ZP_UNUSED(compare); -#endif + _z_lru_cache_insert_slist(cache, node, compare, idx); cache->len++; } static _z_lru_cache_node_t *_z_lru_cache_search_node(_z_lru_cache_t *cache, void *value, _z_lru_val_cmp_f compare) { -#if Z_FEATURE_CACHE_TREE == 1 - return _z_lru_cache_search_tree(cache, value, compare); -#else - return _z_lru_cache_search_list(cache, value, compare); -#endif + size_t idx = 0; + return _z_lru_cache_search_slist(cache, value, compare, &idx); } // Public functions @@ -286,7 +185,7 @@ _z_lru_cache_t _z_lru_cache_init(size_t capacity) { } void *_z_lru_cache_get(_z_lru_cache_t *cache, void *value, _z_lru_val_cmp_f compare) { - // Lookup tree if node exists. + // Lookup if node exists. _z_lru_cache_node_t *node = _z_lru_cache_search_node(cache, value, compare); if (node == NULL) { return NULL; @@ -298,23 +197,35 @@ void *_z_lru_cache_get(_z_lru_cache_t *cache, void *value, _z_lru_val_cmp_f comp z_result_t _z_lru_cache_insert(_z_lru_cache_t *cache, void *value, size_t value_size, _z_lru_val_cmp_f compare) { assert(cache->capacity > 0); + // Init slist + if (cache->slist == NULL) { + cache->slist = (_z_lru_cache_node_t **)z_malloc(cache->capacity * sizeof(void *)); + if (cache->slist == NULL) { + return _Z_ERR_SYSTEM_OUT_OF_MEMORY; + } + memset(cache->slist, 0, cache->capacity * sizeof(void *)); + } // Create node _z_lru_cache_node_t *node = _z_lru_cache_node_create(value, value_size); if (node == NULL) { return _Z_ERR_SYSTEM_OUT_OF_MEMORY; } // Check capacity + size_t *idx = NULL; + size_t del_idx = 0; if (cache->len == cache->capacity) { // Delete lru entry - _z_lru_cache_delete_last(cache, value_size, compare); + del_idx = _z_lru_cache_delete_last(cache, compare); + idx = &del_idx; } // Update the cache - _z_lru_cache_insert_node(cache, node, compare); + _z_lru_cache_insert_node(cache, node, compare, idx); return _Z_RES_OK; } void _z_lru_cache_delete(_z_lru_cache_t *cache) { _z_lru_cache_node_data_t *node = cache->head; + z_free(cache->slist); // Parse list while (node != NULL) { _z_lru_cache_node_t *tmp = node; diff --git a/tests/z_lru_cache_test.c b/tests/z_lru_cache_test.c index 64dc4e7e8..daa9a5345 100644 --- a/tests/z_lru_cache_test.c +++ b/tests/z_lru_cache_test.c @@ -191,12 +191,7 @@ void *stop_task(void *ctx) { *stop_flag = true; return NULL; } -// Results -// size 10: 287720681 list, 294626805 tree (1.024) -// size 20: 207124561 list, 282312820 tree (1.363) -// size 50: 104164238 list, 212367113 tree (2.039) -// size 100: 51666425 list, 198541036 tree (3.843) -// size 1000: 5838653 list, 122717170 tree (21.018) + void test_search_benchmark(void) { _dummy_lru_cache_t dcache = _dummy_lru_cache_init(10); _dummy_t data[10] = {0}; @@ -219,7 +214,7 @@ void test_search_benchmark(void) { get_cnt++; assert(res != NULL); } - printf("Mode: %d, Get count: %ld\n", Z_FEATURE_CACHE_TREE, get_cnt); + printf("Get count: %ld\n", get_cnt); } #endif From 679e386c8d3fb91fe2e06337c167be124c8c7508 Mon Sep 17 00:00:00 2001 From: Jean-Roland Date: Fri, 20 Dec 2024 10:29:57 +0100 Subject: [PATCH 13/15] fix: windows doesn't have ssize_t --- src/collections/lru_cache.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/collections/lru_cache.c b/src/collections/lru_cache.c index aa0f62bd9..f83b079d6 100644 --- a/src/collections/lru_cache.c +++ b/src/collections/lru_cache.c @@ -105,10 +105,10 @@ static void _z_lru_cache_update_list(_z_lru_cache_t *cache, _z_lru_cache_node_t // Sorted list function static _z_lru_cache_node_t *_z_lru_cache_search_slist(_z_lru_cache_t *cache, void *value, _z_lru_val_cmp_f compare, size_t *idx) { - ssize_t l_idx = 0; - ssize_t h_idx = (ssize_t)cache->len - 1; + int l_idx = 0; + int h_idx = (int)cache->len - 1; while (l_idx <= h_idx) { - ssize_t curr_idx = (l_idx + h_idx) / 2; + int curr_idx = (l_idx + h_idx) / 2; int res = compare(_z_lru_cache_node_value(cache->slist[curr_idx]), value); if (res == 0) { *idx = (size_t)curr_idx; @@ -126,7 +126,7 @@ static void _z_lru_cache_sort_slist(_z_lru_cache_node_t **slist, size_t slist_si for (size_t i = 1; i < slist_size; i++) { _z_lru_cache_node_t *node = slist[i]; void *node_val = _z_lru_cache_node_value(node); - ssize_t j = (ssize_t)i - 1; + int j = (int)i - 1; while ((j >= 0) && (compare(_z_lru_cache_node_value(slist[j]), node_val) > 0)) { slist[j + 1] = slist[j]; j--; From 8ea1e651d4f964ff777d23c4a014cf313e170d19 Mon Sep 17 00:00:00 2001 From: Jean-Roland Date: Fri, 20 Dec 2024 11:25:29 +0100 Subject: [PATCH 14/15] feat: add rx cache invalidation --- include/zenoh-pico/collections/lru_cache.h | 2 ++ include/zenoh-pico/session/queryable.h | 1 + include/zenoh-pico/session/subscription.h | 1 + src/collections/lru_cache.c | 18 +++++++++++++++ src/net/primitives.c | 19 ++++++++++++--- src/session/queryable.c | 8 +++++++ src/session/subscription.c | 8 +++++++ tests/z_lru_cache_test.c | 27 +++++++++++++++++++--- 8 files changed, 78 insertions(+), 6 deletions(-) diff --git a/include/zenoh-pico/collections/lru_cache.h b/include/zenoh-pico/collections/lru_cache.h index 1bed89b27..9fb783b8e 100644 --- a/include/zenoh-pico/collections/lru_cache.h +++ b/include/zenoh-pico/collections/lru_cache.h @@ -45,6 +45,7 @@ typedef struct _z_lru_cache_t { _z_lru_cache_t _z_lru_cache_init(size_t capacity); void *_z_lru_cache_get(_z_lru_cache_t *cache, void *value, _z_lru_val_cmp_f compare); z_result_t _z_lru_cache_insert(_z_lru_cache_t *cache, void *value, size_t value_size, _z_lru_val_cmp_f compare); +void _z_lru_cache_clear(_z_lru_cache_t *cache); void _z_lru_cache_delete(_z_lru_cache_t *cache); #define _Z_LRU_CACHE_DEFINE(name, type, compare_f) \ @@ -56,6 +57,7 @@ void _z_lru_cache_delete(_z_lru_cache_t *cache); static inline z_result_t name##_lru_cache_insert(name##_lru_cache_t *cache, type *val) { \ return _z_lru_cache_insert(cache, (void *)val, sizeof(type), compare_f); \ } \ + static inline void name##_lru_cache_clear(name##_lru_cache_t *cache) { _z_lru_cache_clear(cache); } \ static inline void name##_lru_cache_delete(name##_lru_cache_t *cache) { _z_lru_cache_delete(cache); } #ifdef __cplusplus diff --git a/include/zenoh-pico/session/queryable.h b/include/zenoh-pico/session/queryable.h index c92527859..019ce6e1e 100644 --- a/include/zenoh-pico/session/queryable.h +++ b/include/zenoh-pico/session/queryable.h @@ -40,6 +40,7 @@ typedef struct { size_t qle_nb; } _z_queryable_cache_data_t; +void _z_queryable_cache_invalidate(_z_session_t *zn); int _z_queryable_cache_data_compare(const void *first, const void *second); #if Z_FEATURE_QUERYABLE == 1 diff --git a/include/zenoh-pico/session/subscription.h b/include/zenoh-pico/session/subscription.h index a78d23fd8..07c037992 100644 --- a/include/zenoh-pico/session/subscription.h +++ b/include/zenoh-pico/session/subscription.h @@ -43,6 +43,7 @@ typedef struct { size_t sub_nb; } _z_subscription_cache_data_t; +void _z_subscription_cache_invalidate(_z_session_t *zn); int _z_subscription_cache_data_compare(const void *first, const void *second); /*------------------ Subscription ------------------*/ diff --git a/src/collections/lru_cache.c b/src/collections/lru_cache.c index f83b079d6..c306c3d6d 100644 --- a/src/collections/lru_cache.c +++ b/src/collections/lru_cache.c @@ -223,6 +223,24 @@ z_result_t _z_lru_cache_insert(_z_lru_cache_t *cache, void *value, size_t value_ return _Z_RES_OK; } +void _z_lru_cache_clear(_z_lru_cache_t *cache) { + // Reset slist + if (cache->slist != NULL) { + memset(cache->slist, 0, cache->capacity * sizeof(void *)); + } + // Remove nodes + _z_lru_cache_node_data_t *node = cache->head; + while (node != NULL) { + _z_lru_cache_node_t *tmp = node; + _z_lru_cache_node_data_t *node_data = _z_lru_cache_node_data(node); + node = node_data->next; + z_free(tmp); + } + cache->len = 0; + cache->head = NULL; + cache->tail = NULL; +} + void _z_lru_cache_delete(_z_lru_cache_t *cache) { _z_lru_cache_node_data_t *node = cache->head; z_free(cache->slist); diff --git a/src/net/primitives.c b/src/net/primitives.c index e6b9a73d6..dcad5d6b9 100644 --- a/src/net/primitives.c +++ b/src/net/primitives.c @@ -99,13 +99,15 @@ uint16_t _z_declare_resource(_z_session_t *zn, const _z_keyexpr_t *keyexpr) { _z_network_message_t n_msg = _z_n_msg_make_declare(declaration, false, 0); if (_z_send_decalre(zn, &n_msg) == _Z_RES_OK) { ret = id; + // Invalidate cache + _z_subscription_cache_invalidate(zn); + _z_queryable_cache_invalidate(zn); } else { _z_unregister_resource(zn, id, _Z_KEYEXPR_MAPPING_LOCAL); } _z_n_msg_clear(&n_msg); } } - return ret; } @@ -118,8 +120,11 @@ z_result_t _z_undeclare_resource(_z_session_t *zn, uint16_t rid) { _z_declaration_t declaration = _z_make_undecl_keyexpr(rid); _z_network_message_t n_msg = _z_n_msg_make_declare(declaration, false, 0); if (_z_send_undecalre(zn, &n_msg) == _Z_RES_OK) { - _z_unregister_resource(zn, rid, - _Z_KEYEXPR_MAPPING_LOCAL); // Only if message is send, local resource is removed + // Remove local resource + _z_unregister_resource(zn, rid, _Z_KEYEXPR_MAPPING_LOCAL); + // Invalidate cache + _z_subscription_cache_invalidate(zn); + _z_queryable_cache_invalidate(zn); } else { ret = _Z_ERR_TRANSPORT_TX_FAILED; } @@ -273,6 +278,8 @@ _z_subscriber_t _z_declare_subscriber(const _z_session_rc_t *zn, _z_keyexpr_t ke // Fill subscriber ret._entity_id = s._id; ret._zn = _z_session_rc_clone_as_weak(zn); + // Invalidate cache + _z_subscription_cache_invalidate(_Z_RC_IN_VAL(zn)); return ret; } @@ -301,6 +308,8 @@ z_result_t _z_undeclare_subscriber(_z_subscriber_t *sub) { // Only if message is successfully send, local subscription state can be removed _z_undeclare_resource(_Z_RC_IN_VAL(&sub->_zn), _Z_RC_IN_VAL(s)->_key_id); _z_unregister_subscription(_Z_RC_IN_VAL(&sub->_zn), _Z_SUBSCRIBER_KIND_SUBSCRIBER, s); + // Invalidate cache + _z_subscription_cache_invalidate(_Z_RC_IN_VAL(&sub->_zn)); return _Z_RES_OK; } #endif @@ -336,6 +345,8 @@ _z_queryable_t _z_declare_queryable(const _z_session_rc_t *zn, _z_keyexpr_t keye // Fill queryable ret._entity_id = q._id; ret._zn = _z_session_rc_clone_as_weak(zn); + // Invalidate cache + _z_queryable_cache_invalidate(_Z_RC_IN_VAL(zn)); return ret; } @@ -362,6 +373,8 @@ z_result_t _z_undeclare_queryable(_z_queryable_t *qle) { _z_n_msg_clear(&n_msg); // Only if message is successfully send, local queryable state can be removed _z_unregister_session_queryable(_Z_RC_IN_VAL(&qle->_zn), q); + // Invalidate cache + _z_queryable_cache_invalidate(_Z_RC_IN_VAL(&qle->_zn)); return _Z_RES_OK; } diff --git a/src/session/queryable.c b/src/session/queryable.c index a4c7106a0..641afba42 100644 --- a/src/session/queryable.c +++ b/src/session/queryable.c @@ -37,6 +37,14 @@ static inline _z_queryable_cache_data_t _z_queryable_cache_data_null(void) { return ret; } +void _z_queryable_cache_invalidate(_z_session_t *zn) { +#if Z_FEATURE_RX_CACHE == 1 + _z_queryable_lru_cache_clear(&zn->_queryable_cache); +#else + _ZP_UNUSED(zn); +#endif +} + #if Z_FEATURE_RX_CACHE == 1 int _z_queryable_cache_data_compare(const void *first, const void *second) { _z_queryable_cache_data_t *first_data = (_z_queryable_cache_data_t *)first; diff --git a/src/session/subscription.c b/src/session/subscription.c index d4c7ffe40..6c007c148 100644 --- a/src/session/subscription.c +++ b/src/session/subscription.c @@ -38,6 +38,14 @@ static inline _z_subscription_cache_data_t _z_subscription_cache_data_null(void) return ret; } +void _z_subscription_cache_invalidate(_z_session_t *zn) { +#if Z_FEATURE_RX_CACHE == 1 + _z_subscription_lru_cache_clear(&zn->_subscription_cache); +#else + _ZP_UNUSED(zn); +#endif +} + #if Z_FEATURE_RX_CACHE == 1 int _z_subscription_cache_data_compare(const void *first, const void *second) { _z_subscription_cache_data_t *first_data = (_z_subscription_cache_data_t *)first; diff --git a/tests/z_lru_cache_test.c b/tests/z_lru_cache_test.c index daa9a5345..329ec185a 100644 --- a/tests/z_lru_cache_test.c +++ b/tests/z_lru_cache_test.c @@ -49,17 +49,17 @@ void test_lru_init(void) { assert(dcache.len == 0); assert(dcache.head == NULL); assert(dcache.tail == NULL); -#if Z_FEATURE_CACHE_TREE == 1 - assert(dcache.root == NULL); -#endif + assert(dcache.slist == NULL); } void test_lru_cache_insert(void) { _dummy_lru_cache_t dcache = _dummy_lru_cache_init(CACHE_CAPACITY); _dummy_t v0 = {0}; + assert(dcache.slist == NULL); assert(_dummy_lru_cache_get(&dcache, &v0) == NULL); assert(_dummy_lru_cache_insert(&dcache, &v0) == 0); + assert(dcache.slist != NULL); _dummy_t *res = _dummy_lru_cache_get(&dcache, &v0); assert(res != NULL); assert(res->foo == v0.foo); @@ -77,6 +77,26 @@ void test_lru_cache_insert(void) { _dummy_lru_cache_delete(&dcache); } +void test_lru_cache_clear(void) { + _dummy_lru_cache_t dcache = _dummy_lru_cache_init(CACHE_CAPACITY); + + _dummy_t data[CACHE_CAPACITY] = {0}; + for (size_t i = 0; i < CACHE_CAPACITY; i++) { + data[i].foo = (int)i; + assert(_dummy_lru_cache_insert(&dcache, &data[i]) == 0); + } + _dummy_lru_cache_clear(&dcache); + assert(dcache.capacity == CACHE_CAPACITY); + assert(dcache.len == 0); + assert(dcache.slist != NULL); + assert(dcache.head == NULL); + assert(dcache.tail == NULL); + for (size_t i = 0; i < CACHE_CAPACITY; i++) { + assert(_dummy_lru_cache_get(&dcache, &data[i]) == NULL); + } + _dummy_lru_cache_delete(&dcache); +} + void test_lru_cache_deletion(void) { _dummy_lru_cache_t dcache = _dummy_lru_cache_init(CACHE_CAPACITY); @@ -221,6 +241,7 @@ void test_search_benchmark(void) { int main(void) { test_lru_init(); test_lru_cache_insert(); + test_lru_cache_clear(); test_lru_cache_deletion(); test_lru_cache_update(); test_lru_cache_random_val(); From 4caad5f6e350519312ee5810cf855a8e9f7b49f3 Mon Sep 17 00:00:00 2001 From: Jean-Roland Date: Fri, 20 Dec 2024 12:17:49 +0100 Subject: [PATCH 15/15] fix: modular test error --- src/session/queryable.c | 5 ++++- src/session/subscription.c | 2 ++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/session/queryable.c b/src/session/queryable.c index 641afba42..1244bd83f 100644 --- a/src/session/queryable.c +++ b/src/session/queryable.c @@ -269,5 +269,8 @@ void _z_flush_session_queryable(_z_session_t *zn) { _z_session_mutex_unlock(zn); } +#else // Z_FEATURE_QUERYABLE == 0 -#endif +void _z_queryable_cache_invalidate(_z_session_t *zn) { _ZP_UNUSED(zn); } + +#endif // Z_FEATURE_QUERYABLE == 1 diff --git a/src/session/subscription.c b/src/session/subscription.c index 6c007c148..9cdffd6b6 100644 --- a/src/session/subscription.c +++ b/src/session/subscription.c @@ -350,4 +350,6 @@ z_result_t _z_trigger_liveliness_subscriptions_undeclare(_z_session_t *zn, _z_ke return _Z_RES_OK; } +void _z_subscription_cache_invalidate(_z_session_t *zn) { _ZP_UNUSED(zn); } + #endif // Z_FEATURE_SUBSCRIPTION == 1