From 79101213a572b107b6e28e3775fb2a1b710a72c1 Mon Sep 17 00:00:00 2001 From: "Dirk O. Kaar" <19971886+dok-net@users.noreply.github.com> Date: Wed, 5 Jun 2019 06:14:14 +0200 Subject: [PATCH] Use placement new for ETSTimer - no heap fragmentation (#6164) * Use placement new for ETSTimer - no heap fragmentation, new/delete semantics unchanged. * Make change completely invisible to derived classes at compile-time. * Fix "sizeof() incomplete type ETSTimer" error. * C++ reinterpret_cast<> instead of C-style cast. void* instead of uint32_t - fixes x86_64 server compiles. * Simplify casts. * Revert to complete placement new treatment of ETSTimer member. * Cleanup includes * Fix omitted casts * Change per review https://github.com/esp8266/Arduino/pull/6164#pullrequestreview-243583458 * wtf - local compile didn't catch this sloppy mistake * Resolves review https://github.com/esp8266/Arduino/pull/6164#discussion_r290388119 * Reviewer stated that floating point operations are inlined, software operations - reduce number of code spots to one. --- libraries/Ticker/Ticker.cpp | 45 +++++++++++++-------------- libraries/Ticker/Ticker.h | 62 +++++++++++++++++-------------------- 2 files changed, 49 insertions(+), 58 deletions(-) diff --git a/libraries/Ticker/Ticker.cpp b/libraries/Ticker/Ticker.cpp index b53b429da..35ec21cdc 100644 --- a/libraries/Ticker/Ticker.cpp +++ b/libraries/Ticker/Ticker.cpp @@ -1,9 +1,9 @@ -/* +/* Ticker.cpp - esp8266 library that calls functions periodically Copyright (c) 2014 Ivan Grokhotkov. All rights reserved. This file is part of the esp8266 core for Arduino environment. - + This library is free software; you can redistribute it and/or modify it under the terms of the GNU Lesser General Public License as published by the Free Software Foundation; either @@ -19,21 +19,20 @@ Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA */ -#include -#include - #include "c_types.h" #include "eagle_soc.h" -#include "ets_sys.h" #include "osapi.h" -static const int ONCE = 0; -static const int REPEAT = 1; - #include "Ticker.h" +namespace +{ + constexpr int ONCE = 0; + constexpr int REPEAT = 1; +} + Ticker::Ticker() -: _timer(nullptr) + : _timer(nullptr) { } @@ -42,7 +41,12 @@ Ticker::~Ticker() detach(); } -void Ticker::_attach_ms(uint32_t milliseconds, bool repeat, callback_with_arg_t callback, uint32_t arg) +void Ticker::_attach_s(float seconds, bool repeat, callback_with_arg_t callback, void* arg) +{ + _attach_ms(1000 * seconds, repeat, callback, arg); +} + +void Ticker::_attach_ms(uint32_t milliseconds, bool repeat, callback_with_arg_t callback, void* arg) { if (_timer) { @@ -50,11 +54,11 @@ void Ticker::_attach_ms(uint32_t milliseconds, bool repeat, callback_with_arg_t } else { - _timer = new ETSTimer; + _timer = &_etsTimer; } - os_timer_setfn(_timer, reinterpret_cast(callback), reinterpret_cast(arg)); - os_timer_arm(_timer, milliseconds, (repeat)?REPEAT:ONCE); + os_timer_setfn(_timer, callback, arg); + os_timer_arm(_timer, milliseconds, (repeat) ? REPEAT : ONCE); } void Ticker::detach() @@ -63,25 +67,18 @@ void Ticker::detach() return; os_timer_disarm(_timer); - delete _timer; _timer = nullptr; _callback_function = nullptr; } bool Ticker::active() const { - return (bool)_timer; + return _timer; } void Ticker::_static_callback(void* arg) { - Ticker* _this = (Ticker*)arg; - if (_this == nullptr) - { - return; - } - if (_this->_callback_function) - { + Ticker* _this = reinterpret_cast(arg); + if (_this && _this->_callback_function) _this->_callback_function(); - } } diff --git a/libraries/Ticker/Ticker.h b/libraries/Ticker/Ticker.h index 8ecc47581..f8e71721c 100644 --- a/libraries/Ticker/Ticker.h +++ b/libraries/Ticker/Ticker.h @@ -1,9 +1,9 @@ -/* +/* Ticker.h - esp8266 library that calls functions periodically Copyright (c) 2014 Ivan Grokhotkov. All rights reserved. This file is part of the esp8266 core for Arduino environment. - + This library is free software; you can redistribute it and/or modify it under the terms of the GNU Lesser General Public License as published by the Free Software Foundation; either @@ -22,22 +22,16 @@ #ifndef TICKER_H #define TICKER_H -#include -#include -#include #include #include - -extern "C" { - typedef struct _ETSTIMER_ ETSTimer; -} +#include class Ticker { public: Ticker(); ~Ticker(); - typedef void (*callback_t)(void); + typedef void (*callback_with_arg_t)(void*); typedef std::function callback_function_t; @@ -48,8 +42,8 @@ public: void attach(float seconds, callback_function_t callback) { - _callback_function = callback; - attach(seconds, _static_callback, (void*)this); + _callback_function = std::move(callback); + _attach_s(seconds, true, _static_callback, this); } void attach_ms_scheduled(uint32_t milliseconds, callback_function_t callback) @@ -59,27 +53,25 @@ public: void attach_ms(uint32_t milliseconds, callback_function_t callback) { - _callback_function = callback; - attach_ms(milliseconds, _static_callback, (void*)this); + _callback_function = std::move(callback); + _attach_ms(milliseconds, true, _static_callback, this); } template void attach(float seconds, void (*callback)(TArg), TArg arg) { - static_assert(sizeof(TArg) <= sizeof(uint32_t), "attach() callback argument size must be <= 4 bytes"); + static_assert(sizeof(TArg) <= sizeof(void*), "attach() callback argument size must be <= sizeof(void*)"); // C-cast serves two purposes: // static_cast for smaller integer types, // reinterpret_cast + const_cast for pointer types - uint32_t arg32 = (uint32_t)arg; - _attach_ms(seconds * 1000, true, reinterpret_cast(callback), arg32); + _attach_s(seconds, true, reinterpret_cast(callback), (void*)arg); } template void attach_ms(uint32_t milliseconds, void (*callback)(TArg), TArg arg) { - static_assert(sizeof(TArg) <= sizeof(uint32_t), "attach_ms() callback argument size must be <= 4 bytes"); - uint32_t arg32 = (uint32_t)arg; - _attach_ms(milliseconds, true, reinterpret_cast(callback), arg32); + static_assert(sizeof(TArg) <= sizeof(void*), "attach() callback argument size must be <= sizeof(void*)"); + _attach_ms(milliseconds, true, reinterpret_cast(callback), (void*)arg); } void once_scheduled(float seconds, callback_function_t callback) @@ -89,8 +81,8 @@ public: void once(float seconds, callback_function_t callback) { - _callback_function = callback; - once(seconds, _static_callback, (void*)this); + _callback_function = std::move(callback); + _attach_s(seconds, false, _static_callback, this); } void once_ms_scheduled(uint32_t milliseconds, callback_function_t callback) @@ -100,36 +92,38 @@ public: void once_ms(uint32_t milliseconds, callback_function_t callback) { - _callback_function = callback; - once_ms(milliseconds, _static_callback, (void*)this); + _callback_function = std::move(callback); + _attach_ms(milliseconds, false, _static_callback, this); } template void once(float seconds, void (*callback)(TArg), TArg arg) { - static_assert(sizeof(TArg) <= sizeof(uint32_t), "attach() callback argument size must be <= 4 bytes"); - uint32_t arg32 = (uint32_t)(arg); - _attach_ms(seconds * 1000, false, reinterpret_cast(callback), arg32); + static_assert(sizeof(TArg) <= sizeof(void*), "attach() callback argument size must be <= sizeof(void*)"); + _attach_s(seconds, false, reinterpret_cast(callback), (void*)arg); } template void once_ms(uint32_t milliseconds, void (*callback)(TArg), TArg arg) { - static_assert(sizeof(TArg) <= sizeof(uint32_t), "attach_ms() callback argument size must be <= 4 bytes"); - uint32_t arg32 = (uint32_t)(arg); - _attach_ms(milliseconds, false, reinterpret_cast(callback), arg32); + static_assert(sizeof(TArg) <= sizeof(void*), "attach() callback argument size must be <= sizeof(void*)"); + _attach_ms(milliseconds, false, reinterpret_cast(callback), (void*)arg); } void detach(); bool active() const; -protected: - void _attach_ms(uint32_t milliseconds, bool repeat, callback_with_arg_t callback, uint32_t arg); - static void _static_callback (void* arg); - protected: + void _attach_ms(uint32_t milliseconds, bool repeat, callback_with_arg_t callback, void* arg); + static void _static_callback(void* arg); + ETSTimer* _timer; callback_function_t _callback_function = nullptr; + +private: + void _attach_s(float seconds, bool repeat, callback_with_arg_t callback, void* arg); + //char _etsTimerMem[sizeof(ETSTimer)]; + ETSTimer _etsTimer; };