Skip to content

Commit 7910121

Browse files
dok-netdevyte
authored andcommitted
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 #6164 (review) * wtf - local compile didn't catch this sloppy mistake * Resolves review #6164 (comment) * Reviewer stated that floating point operations are inlined, software operations - reduce number of code spots to one.
1 parent 8859b81 commit 7910121

File tree

2 files changed

+49
-58
lines changed

2 files changed

+49
-58
lines changed

libraries/Ticker/Ticker.cpp

+21-24
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
1-
/*
1+
/*
22
Ticker.cpp - esp8266 library that calls functions periodically
33
44
Copyright (c) 2014 Ivan Grokhotkov. All rights reserved.
55
This file is part of the esp8266 core for Arduino environment.
6-
6+
77
This library is free software; you can redistribute it and/or
88
modify it under the terms of the GNU Lesser General Public
99
License as published by the Free Software Foundation; either
@@ -19,21 +19,20 @@
1919
Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
2020
*/
2121

22-
#include <stddef.h>
23-
#include <stdint.h>
24-
2522
#include "c_types.h"
2623
#include "eagle_soc.h"
27-
#include "ets_sys.h"
2824
#include "osapi.h"
2925

30-
static const int ONCE = 0;
31-
static const int REPEAT = 1;
32-
3326
#include "Ticker.h"
3427

28+
namespace
29+
{
30+
constexpr int ONCE = 0;
31+
constexpr int REPEAT = 1;
32+
}
33+
3534
Ticker::Ticker()
36-
: _timer(nullptr)
35+
: _timer(nullptr)
3736
{
3837
}
3938

@@ -42,19 +41,24 @@ Ticker::~Ticker()
4241
detach();
4342
}
4443

45-
void Ticker::_attach_ms(uint32_t milliseconds, bool repeat, callback_with_arg_t callback, uint32_t arg)
44+
void Ticker::_attach_s(float seconds, bool repeat, callback_with_arg_t callback, void* arg)
45+
{
46+
_attach_ms(1000 * seconds, repeat, callback, arg);
47+
}
48+
49+
void Ticker::_attach_ms(uint32_t milliseconds, bool repeat, callback_with_arg_t callback, void* arg)
4650
{
4751
if (_timer)
4852
{
4953
os_timer_disarm(_timer);
5054
}
5155
else
5256
{
53-
_timer = new ETSTimer;
57+
_timer = &_etsTimer;
5458
}
5559

56-
os_timer_setfn(_timer, reinterpret_cast<ETSTimerFunc*>(callback), reinterpret_cast<void*>(arg));
57-
os_timer_arm(_timer, milliseconds, (repeat)?REPEAT:ONCE);
60+
os_timer_setfn(_timer, callback, arg);
61+
os_timer_arm(_timer, milliseconds, (repeat) ? REPEAT : ONCE);
5862
}
5963

6064
void Ticker::detach()
@@ -63,25 +67,18 @@ void Ticker::detach()
6367
return;
6468

6569
os_timer_disarm(_timer);
66-
delete _timer;
6770
_timer = nullptr;
6871
_callback_function = nullptr;
6972
}
7073

7174
bool Ticker::active() const
7275
{
73-
return (bool)_timer;
76+
return _timer;
7477
}
7578

7679
void Ticker::_static_callback(void* arg)
7780
{
78-
Ticker* _this = (Ticker*)arg;
79-
if (_this == nullptr)
80-
{
81-
return;
82-
}
83-
if (_this->_callback_function)
84-
{
81+
Ticker* _this = reinterpret_cast<Ticker*>(arg);
82+
if (_this && _this->_callback_function)
8583
_this->_callback_function();
86-
}
8784
}

libraries/Ticker/Ticker.h

+28-34
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
1-
/*
1+
/*
22
Ticker.h - esp8266 library that calls functions periodically
33
44
Copyright (c) 2014 Ivan Grokhotkov. All rights reserved.
55
This file is part of the esp8266 core for Arduino environment.
6-
6+
77
This library is free software; you can redistribute it and/or
88
modify it under the terms of the GNU Lesser General Public
99
License as published by the Free Software Foundation; either
@@ -22,22 +22,16 @@
2222
#ifndef TICKER_H
2323
#define TICKER_H
2424

25-
#include <stdint.h>
26-
#include <stdbool.h>
27-
#include <stddef.h>
2825
#include <functional>
2926
#include <Schedule.h>
30-
31-
extern "C" {
32-
typedef struct _ETSTIMER_ ETSTimer;
33-
}
27+
#include <ets_sys.h>
3428

3529
class Ticker
3630
{
3731
public:
3832
Ticker();
3933
~Ticker();
40-
typedef void (*callback_t)(void);
34+
4135
typedef void (*callback_with_arg_t)(void*);
4236
typedef std::function<void(void)> callback_function_t;
4337

@@ -48,8 +42,8 @@ class Ticker
4842

4943
void attach(float seconds, callback_function_t callback)
5044
{
51-
_callback_function = callback;
52-
attach(seconds, _static_callback, (void*)this);
45+
_callback_function = std::move(callback);
46+
_attach_s(seconds, true, _static_callback, this);
5347
}
5448

5549
void attach_ms_scheduled(uint32_t milliseconds, callback_function_t callback)
@@ -59,27 +53,25 @@ class Ticker
5953

6054
void attach_ms(uint32_t milliseconds, callback_function_t callback)
6155
{
62-
_callback_function = callback;
63-
attach_ms(milliseconds, _static_callback, (void*)this);
56+
_callback_function = std::move(callback);
57+
_attach_ms(milliseconds, true, _static_callback, this);
6458
}
6559

6660
template<typename TArg>
6761
void attach(float seconds, void (*callback)(TArg), TArg arg)
6862
{
69-
static_assert(sizeof(TArg) <= sizeof(uint32_t), "attach() callback argument size must be <= 4 bytes");
63+
static_assert(sizeof(TArg) <= sizeof(void*), "attach() callback argument size must be <= sizeof(void*)");
7064
// C-cast serves two purposes:
7165
// static_cast for smaller integer types,
7266
// reinterpret_cast + const_cast for pointer types
73-
uint32_t arg32 = (uint32_t)arg;
74-
_attach_ms(seconds * 1000, true, reinterpret_cast<callback_with_arg_t>(callback), arg32);
67+
_attach_s(seconds, true, reinterpret_cast<callback_with_arg_t>(callback), (void*)arg);
7568
}
7669

7770
template<typename TArg>
7871
void attach_ms(uint32_t milliseconds, void (*callback)(TArg), TArg arg)
7972
{
80-
static_assert(sizeof(TArg) <= sizeof(uint32_t), "attach_ms() callback argument size must be <= 4 bytes");
81-
uint32_t arg32 = (uint32_t)arg;
82-
_attach_ms(milliseconds, true, reinterpret_cast<callback_with_arg_t>(callback), arg32);
73+
static_assert(sizeof(TArg) <= sizeof(void*), "attach() callback argument size must be <= sizeof(void*)");
74+
_attach_ms(milliseconds, true, reinterpret_cast<callback_with_arg_t>(callback), (void*)arg);
8375
}
8476

8577
void once_scheduled(float seconds, callback_function_t callback)
@@ -89,8 +81,8 @@ class Ticker
8981

9082
void once(float seconds, callback_function_t callback)
9183
{
92-
_callback_function = callback;
93-
once(seconds, _static_callback, (void*)this);
84+
_callback_function = std::move(callback);
85+
_attach_s(seconds, false, _static_callback, this);
9486
}
9587

9688
void once_ms_scheduled(uint32_t milliseconds, callback_function_t callback)
@@ -100,36 +92,38 @@ class Ticker
10092

10193
void once_ms(uint32_t milliseconds, callback_function_t callback)
10294
{
103-
_callback_function = callback;
104-
once_ms(milliseconds, _static_callback, (void*)this);
95+
_callback_function = std::move(callback);
96+
_attach_ms(milliseconds, false, _static_callback, this);
10597
}
10698

10799
template<typename TArg>
108100
void once(float seconds, void (*callback)(TArg), TArg arg)
109101
{
110-
static_assert(sizeof(TArg) <= sizeof(uint32_t), "attach() callback argument size must be <= 4 bytes");
111-
uint32_t arg32 = (uint32_t)(arg);
112-
_attach_ms(seconds * 1000, false, reinterpret_cast<callback_with_arg_t>(callback), arg32);
102+
static_assert(sizeof(TArg) <= sizeof(void*), "attach() callback argument size must be <= sizeof(void*)");
103+
_attach_s(seconds, false, reinterpret_cast<callback_with_arg_t>(callback), (void*)arg);
113104
}
114105

115106
template<typename TArg>
116107
void once_ms(uint32_t milliseconds, void (*callback)(TArg), TArg arg)
117108
{
118-
static_assert(sizeof(TArg) <= sizeof(uint32_t), "attach_ms() callback argument size must be <= 4 bytes");
119-
uint32_t arg32 = (uint32_t)(arg);
120-
_attach_ms(milliseconds, false, reinterpret_cast<callback_with_arg_t>(callback), arg32);
109+
static_assert(sizeof(TArg) <= sizeof(void*), "attach() callback argument size must be <= sizeof(void*)");
110+
_attach_ms(milliseconds, false, reinterpret_cast<callback_with_arg_t>(callback), (void*)arg);
121111
}
122112

123113
void detach();
124114
bool active() const;
125115

126-
protected:
127-
void _attach_ms(uint32_t milliseconds, bool repeat, callback_with_arg_t callback, uint32_t arg);
128-
static void _static_callback (void* arg);
129-
130116
protected:
117+
void _attach_ms(uint32_t milliseconds, bool repeat, callback_with_arg_t callback, void* arg);
118+
static void _static_callback(void* arg);
119+
131120
ETSTimer* _timer;
132121
callback_function_t _callback_function = nullptr;
122+
123+
private:
124+
void _attach_s(float seconds, bool repeat, callback_with_arg_t callback, void* arg);
125+
//char _etsTimerMem[sizeof(ETSTimer)];
126+
ETSTimer _etsTimer;
133127
};
134128

135129

0 commit comments

Comments
 (0)