Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Limit Timer #506

Merged
merged 7 commits into from
Jan 11, 2021
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
136 changes: 136 additions & 0 deletions src/utils/LimitTimer.cxxtest
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
/** \copyright
* Copyright (c) 2021, Stuart Baker
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are met:
*
* - Redistributions of source code must retain the above copyright notice,
* this list of conditions and the following disclaimer.
*
* - Redistributions in binary form must reproduce the above copyright notice,
* this list of conditions and the following disclaimer in the documentation
* and/or other materials provided with the distribution.
*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
* AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
* ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE
* LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
* CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
* SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
* INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
* CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
* ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
* POSSIBILITY OF SUCH DAMAGE.
*
* @file LimitTimer.cxxtest
*
* Unit tests for LimitTimer.
*
* @author Stuart Baker
* @date 10 January 2021
*/

#include "utils/test_main.hxx"

#include "utils/LimitTimer.hxx"
#include "os/FakeClock.hxx"

class MockCallback
{
public:
MOCK_METHOD0(callback, void());
};

class MyLimitTimer
{
public:
/// Constructor.
/// @param update_delay_msec cooldown time delay in milliseconds
/// @param max_tokens number of available tokens
MyLimitTimer(uint16_t update_delay_msec,
uint8_t max_tokens)
: mockCallback_()
, limitTimer_(&g_executor, update_delay_msec, max_tokens,
std::bind(&MockCallback::callback, &mockCallback_))
{
}

/// Destructor.
~MyLimitTimer()
{
wait_for_main_executor();
while (!g_executor.active_timers()->empty())
{
sleep_helper(20);
}
}

/// Helper function for sleeping.
/// @param clk fake clock or nullptr if no fake clock exists
/// @param len_msec how long to sleep
/// @param step_msec what granularity to use for sleeping wiht fake clock.
void sleep_helper(unsigned len_msec, unsigned step_msec = 50)
{
for (unsigned i = 0; i < len_msec; i += step_msec)
{
clk_.advance(MSEC_TO_NSEC(step_msec));
wait_for_main_executor();
}
}

FakeClock clk_;

::testing::StrictMock<MockCallback> mockCallback_;
LimitTimer limitTimer_;
};

TEST(LimitTimerTest, Create)
{
MyLimitTimer(200, 3);
}

TEST(LimitTimerTest, TryTake)
{
MyLimitTimer lt(200, 3);

::testing::MockFunction<void(string check_point_name)> check;

::testing::InSequence s;

// flush out the tokens
EXPECT_CALL(lt.mockCallback_, callback()).Times(0);
EXPECT_TRUE(lt.limitTimer_.try_take());
EXPECT_TRUE(lt.limitTimer_.try_take());
EXPECT_TRUE(lt.limitTimer_.try_take());
EXPECT_CALL(check, Call("1"));
check.Call("1");

// try to pull one more token out (that is not available)
EXPECT_CALL(lt.mockCallback_, callback()).Times(0);
EXPECT_FALSE(lt.limitTimer_.try_take());
EXPECT_CALL(check, Call("2"));
check.Call("2");

// verify the callback after timeout
EXPECT_CALL(lt.mockCallback_, callback()).Times(1);
EXPECT_CALL(check, Call("3"));
lt.sleep_helper(200);
check.Call("3");

// timer should still be running as bucket gets refilled
lt.sleep_helper(200);
EXPECT_FALSE(g_executor.active_timers()->empty());

// pull a token from the bucket
lt.limitTimer_.take_no_callback();

// timer should still be running as bucket gets refilled
lt.sleep_helper(200);
EXPECT_FALSE(g_executor.active_timers()->empty());

// timer should stop running once bucket gets refilled
lt.sleep_helper(200);
EXPECT_TRUE(g_executor.active_timers()->empty());
}
132 changes: 132 additions & 0 deletions src/utils/LimitTimer.hxx
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
/** @copyright
* Copyright (c) 2020, Balazs Racz; 2021 Stuart Baker
* All rights reserved
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are met:
*
* - Redistributions of source code must retain the above copyright notice,
* this list of conditions and the following disclaimer.
*
* - Redistributions in binary form must reproduce the above copyright notice,
* this list of conditions and the following disclaimer in the documentation
* and/or other materials provided with the distribution.
*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
* AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
* ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE
* LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
* CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
* SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
* INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
* CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
* ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
* POSSIBILITY OF SUCH DAMAGE.
*
* @file LimitTimer.hxx
*
* Limits the number of updates per unit time. Initial version written by
* Balazs Racz, modified for generalization by Stuart Baker
*
* @author Balazs Racz, Stuart Baker
* @date 9 January 2021
*/

#include "executor/Timer.hxx"

/// This timer takes care of limiting the number of speed updates we send
/// out in a second. It is a token bucket filter.
class LimitTimer : public Timer
{
public:
/// Constructor.
/// @param ex executor to run on
/// @param update_delay_msec cooldown time delay in milliseconds
/// @param max_tokens number of available tokens
/// @param callback callback called once after cooldown time delay
LimitTimer(ExecutorBase *ex, uint16_t update_delay_msec, uint8_t max_tokens,
std::function<void()> callback)
: Timer(ex->active_timers())
, updateDelayMsec_(update_delay_msec)
, bucket_(max_tokens)
, bucketMax_(max_tokens)
, callback_(callback)
{
HASSERT(callback);
}

/// Destructor.
~LimitTimer()
{
cancel();
}

/// Attempts to take a token out of the bucket.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a comment that this call must be made from the executor passed in on the constructor.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

/// @return true if the take is successful, false if there are no available
/// tokens, in which case there will be an internal key event
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

replace 'internal key event' with 'callback'

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

/// generated when tokens become available.
bool try_take()
{
if (bucket_ == bucketMax_)
{
start(MSEC_TO_NSEC(updateDelayMsec_));
}
if (bucket_ > 0)
{
--bucket_;
return true;
}
else
{
needUpdate_ = 1;
return false;
}
}

/// Takes one entry from the bucket, and does not give a callback if
/// there is no entry left.
void take_no_callback()
{
if (bucket_ > 0)
{
--bucket_;
}
}

private:
/// Callback from the timer infrastructure. Called periodically.
long long timeout() override
{
++bucket_;
if (needUpdate_)
{
callback_();
needUpdate_ = 0;
}
if (bucket_ >= bucketMax_)
{
return NONE;
}
else
{
return RESTART;
}
}

/// cooldown delay in msec
uint16_t updateDelayMsec_;

/// number of available tokens
uint8_t bucket_ ;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you squeeze these into an uint16_t bucket_ : 7; , make bucketMax the same, and needUpdate with just one bit?
Then the data fields fit into a single 32 bit memory. I think we may have up to 62 of these in RAM.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are correct, there are up to 61 of these in one of our use cases (index 0 is broadcast, index 1 is reserved). I had considered this myself, and since you are are board, let's do it. Fixed.

Don't forget that we have an std::function, so we don't quite fit into 32-bits of memory.


/// maximum number of tokens in the bucket
uint8_t bucketMax_;

/// if non-zero, wake up parent when token is available.
uint8_t needUpdate_ {0};

/// callback after cooldown period.
std::function<void()> callback_;
};