From 5f5dca480430bb9af3056768ae05b17e80af5f5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beat=20K=C3=BCng?= Date: Mon, 21 Aug 2017 10:53:58 +0200 Subject: [PATCH] vdev: replace static list with an std::map VDev::getDev() is used in px4_access, which is used in orb_exists. And if the topic does not exist, it iterates over all 500 indexes, which is slow. It was slow even if the topic existed, the map reduces runtime from linear to logarithmic (there are around 80 items in the container). This is only used on posix. --- src/drivers/device/vdev.cpp | 113 ++++++++++++------------------------ 1 file changed, 37 insertions(+), 76 deletions(-) diff --git a/src/drivers/device/vdev.cpp b/src/drivers/device/vdev.cpp index 7db1cde921..99d9acf1af 100644 --- a/src/drivers/device/vdev.cpp +++ b/src/drivers/device/vdev.cpp @@ -44,33 +44,20 @@ #include #include #include +#include +#include #include "DevMgr.hpp" using namespace DriverFramework; +using namespace std; namespace device { int px4_errno; -struct px4_dev_t { - char *name; - void *cdev; - - px4_dev_t(const char *n, void *c) : cdev(c) - { - name = strdup(n); - } - - ~px4_dev_t() { free(name); } - -private: - px4_dev_t() {} -}; - -#define PX4_MAX_DEV 500 -static px4_dev_t *devmap[PX4_MAX_DEV]; +static map devmap; pthread_mutex_t devmutex = PTHREAD_MUTEX_INITIALIZER; @@ -137,39 +124,27 @@ int VDev::register_driver(const char *name, void *data) { PX4_DEBUG("VDev::register_driver %s", name); - int ret = -ENOSPC; + int ret = 0; if (name == nullptr || data == nullptr) { return -EINVAL; } - // Make sure the device does not already exist - // FIXME - convert this to a map for efficiency - pthread_mutex_lock(&devmutex); - for (int i = 0; i < PX4_MAX_DEV; ++i) { - if (devmap[i] && (strcmp(devmap[i]->name, name) == 0)) { - pthread_mutex_unlock(&devmutex); - return -EEXIST; - } + // Make sure the device does not already exist + auto item = devmap.find(name); + + if (item != devmap.end()) { + pthread_mutex_unlock(&devmutex); + return -EEXIST; } - for (int i = 0; i < PX4_MAX_DEV; ++i) { - if (devmap[i] == nullptr) { - devmap[i] = new px4_dev_t(name, (void *)data); - PX4_DEBUG("Registered DEV %s", name); - ret = PX4_OK; - break; - } - } + devmap[name] = (void *)data; + PX4_DEBUG("Registered DEV %s", name); pthread_mutex_unlock(&devmutex); - if (ret != PX4_OK) { - PX4_ERR("No free devmap entries - increase PX4_MAX_DEV"); - } - return ret; } @@ -185,14 +160,9 @@ VDev::unregister_driver(const char *name) pthread_mutex_lock(&devmutex); - for (int i = 0; i < PX4_MAX_DEV; ++i) { - if (devmap[i] && (strcmp(name, devmap[i]->name) == 0)) { - delete devmap[i]; - devmap[i] = nullptr; - PX4_DEBUG("Unregistered DEV %s", name); - ret = PX4_OK; - break; - } + if (devmap.erase(name) > 0) { + PX4_DEBUG("Unregistered DEV %s", name); + ret = 0; } pthread_mutex_unlock(&devmutex); @@ -206,22 +176,19 @@ VDev::unregister_class_devname(const char *class_devname, unsigned class_instanc PX4_DEBUG("VDev::unregister_class_devname"); char name[32]; snprintf(name, sizeof(name), "%s%u", class_devname, class_instance); + int ret = -EINVAL; + PX4_WARN("unregistering class %s", name); pthread_mutex_lock(&devmutex); - for (int i = 0; i < PX4_MAX_DEV; ++i) { - if (devmap[i] && strcmp(devmap[i]->name, name) == 0) { - delete devmap[i]; - PX4_DEBUG("Unregistered class DEV %s", name); - devmap[i] = nullptr; - pthread_mutex_unlock(&devmutex); - return PX4_OK; - } + if (devmap.erase(name) > 0) { + PX4_DEBUG("Unregistered class DEV %s", name); + ret = 0; } pthread_mutex_unlock(&devmutex); - return -EINVAL; + return ret; } int @@ -535,18 +502,14 @@ VDev::remove_poll_waiter(px4_pollfd_struct_t *fds) VDev *VDev::getDev(const char *path) { PX4_DEBUG("VDev::getDev"); - int i = 0; pthread_mutex_lock(&devmutex); - for (; i < PX4_MAX_DEV; ++i) { - //if (devmap[i]) { - // printf("%s %s\n", devmap[i]->name, path); - //} - if (devmap[i] && (strcmp(devmap[i]->name, path) == 0)) { - pthread_mutex_unlock(&devmutex); - return (VDev *)(devmap[i]->cdev); - } + auto item = devmap.find(path); + + if (item != devmap.end()) { + pthread_mutex_unlock(&devmutex); + return (VDev *)item->second; } pthread_mutex_unlock(&devmutex); @@ -561,9 +524,9 @@ void VDev::showDevices() pthread_mutex_lock(&devmutex); - for (; i < PX4_MAX_DEV; ++i) { - if (devmap[i] && strncmp(devmap[i]->name, "/dev/", 5) == 0) { - PX4_INFO(" %s", devmap[i]->name); + for (const auto &dev : devmap) { + if (strncmp(dev.first.c_str(), "/dev/", 5) == 0) { + PX4_INFO(" %s", dev.first.c_str()); } } @@ -586,14 +549,13 @@ void VDev::showDevices() void VDev::showTopics() { - int i = 0; PX4_INFO("Devices:"); pthread_mutex_lock(&devmutex); - for (; i < PX4_MAX_DEV; ++i) { - if (devmap[i] && strncmp(devmap[i]->name, "/obj/", 5) == 0) { - PX4_INFO(" %s", devmap[i]->name); + for (const auto &dev : devmap) { + if (strncmp(dev.first.c_str(), "/obj/", 5) == 0) { + PX4_INFO(" %s", dev.first.c_str()); } } @@ -602,15 +564,14 @@ void VDev::showTopics() void VDev::showFiles() { - int i = 0; PX4_INFO("Files:"); pthread_mutex_lock(&devmutex); - for (; i < PX4_MAX_DEV; ++i) { - if (devmap[i] && strncmp(devmap[i]->name, "/obj/", 5) != 0 && - strncmp(devmap[i]->name, "/dev/", 5) != 0) { - PX4_INFO(" %s", devmap[i]->name); + for (const auto &dev : devmap) { + if (strncmp(dev.first.c_str(), "/obj/", 5) != 0 && + strncmp(dev.first.c_str(), "/dev/", 5) != 0) { + PX4_INFO(" %s", dev.first.c_str()); } }