-
Notifications
You must be signed in to change notification settings - Fork 38
Dan sliding window #42
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,79 @@ | ||
#pragma once | ||
|
||
#include <deque> | ||
#include <chrono> | ||
#include <vector> | ||
#include <stdexcept> | ||
|
||
struct WindowConfig | ||
{ | ||
std::chrono::seconds mDuration = std::chrono::seconds(10); | ||
int mMaxNumMessages = 5; | ||
}; | ||
|
||
template<class Message> | ||
struct TimestampedMessage | ||
{ | ||
std::chrono::time_point<std::chrono::steady_clock> mTimestamp; | ||
Message mMessage; | ||
}; | ||
|
||
template<class Message> | ||
class Buffer | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that this Buffer is effectively a sliding window buffer, and the other Sliding Window class is the per user throttler |
||
{ | ||
public: | ||
using value_type = Message; | ||
|
||
Buffer(const WindowConfig& conf); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. explicit |
||
void TryToAddMessage(const Message& message); | ||
std::vector<Message> DumpMessages() const; | ||
|
||
private: | ||
bool CanAddMessage(std::chrono::time_point<std::chrono::steady_clock> currentTime) const; | ||
|
||
WindowConfig mWindowConfig; | ||
std::deque<TimestampedMessage<Message> > mTimestampedMessages; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not, although it is not mandatory to copy the messages themselves if you only want to throttle. You only need the timestamps. Depending on the type, the message can be quite big and expensive to copy. |
||
}; | ||
|
||
template<class Message> | ||
Buffer<Message>::Buffer(const WindowConfig& conf) : | ||
mWindowConfig(conf) | ||
{ | ||
if (conf.mMaxNumMessages <= 0) | ||
throw std::runtime_error("Invalid configuration: max number of messages per time window must be positive!"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good that you check this, although a better place should be in WindowConfig's ctor. |
||
} | ||
|
||
template<class Message> | ||
bool Buffer<Message>::CanAddMessage(std::chrono::time_point<std::chrono::steady_clock> currentTime) const | ||
{ | ||
if (mTimestampedMessages.size() >= mWindowConfig.mMaxNumMessages) | ||
{ | ||
auto timestampAtStartOfWindow = mTimestampedMessages[ | ||
mTimestampedMessages.size() - mWindowConfig.mMaxNumMessages].mTimestamp; | ||
return (timestampAtStartOfWindow < currentTime - mWindowConfig.mDuration); | ||
} | ||
return true; | ||
} | ||
|
||
template<class Message> | ||
void Buffer<Message>::TryToAddMessage(const Message& message) | ||
{ | ||
auto currentTime = std::chrono::steady_clock::now(); | ||
if (CanAddMessage(currentTime)) | ||
{ | ||
TimestampedMessage<Message> tsMsg; | ||
tsMsg.mMessage = message; | ||
tsMsg.mTimestamp = currentTime; | ||
mTimestampedMessages.push_back(tsMsg); | ||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems that this container grows infinitely. This cant be right - why would you keep more than the last N timetamped messages? |
||
|
||
template<class Message> | ||
std::vector<Message> Buffer<Message>::DumpMessages() const | ||
{ | ||
std::vector<Message> result; | ||
for (auto tsMsg : mTimestampedMessages) | ||
result.push_back(tsMsg.mMessage); | ||
|
||
return result; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
cmake_minimum_required(VERSION 2.8) | ||
|
||
set(sources | ||
main.cc | ||
SlidingWindow.h | ||
Buffer.h | ||
) | ||
|
||
add_executable(main ${sources}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
#pragma once | ||
|
||
#include <map> | ||
#include <queue> | ||
#include <chrono> | ||
#include <vector> | ||
|
||
#include "Buffer.h" | ||
|
||
template<class Message> | ||
class SlidingWindow | ||
{ | ||
public: | ||
using value_type = Message; | ||
|
||
SlidingWindow(const WindowConfig& conf); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. explicit |
||
void Send(int clientId, const Message& message); | ||
std::vector<Message> DumpMessages(int clientId) const; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is potentially a very expensive operation as you would copy all messages. A better patern is to visit elements (visitor). |
||
|
||
private: | ||
WindowConfig mWindowConfig; | ||
std::map<int, Buffer<Message> > mClientBuffers; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if it doesnt need to be ordered, its better to use unordered_map There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good to see that you decoupled the per user class and the sliding window buffer |
||
}; | ||
|
||
template<class Message> | ||
SlidingWindow<Message>::SlidingWindow(const WindowConfig& conf) : | ||
mWindowConfig(conf) | ||
{ | ||
if (conf.mMaxNumMessages <= 0) | ||
throw std::runtime_error("Invalid configuration: max number of messages per time window must be positive!"); | ||
} | ||
|
||
template<class Message> | ||
void SlidingWindow<Message>::Send(int clientId, const Message& message) | ||
{ | ||
if (mClientBuffers.count(clientId) == 0) | ||
mClientBuffers.insert(std::pair<int, Buffer<Message> >(clientId, Buffer<Message>(mWindowConfig))); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. emplace |
||
|
||
Buffer<Message>& buffer = mClientBuffers.at(clientId); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You are doing the lookup twice |
||
buffer.TryToAddMessage(message); | ||
} | ||
|
||
template<class Message> | ||
std::vector<Message> SlidingWindow<Message>::DumpMessages(int clientId) const | ||
{ | ||
auto buffer = mClientBuffers.find(clientId); | ||
if (buffer != mClientBuffers.end()) | ||
return buffer->second.DumpMessages(); | ||
else | ||
return std::vector<Message>(); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,84 @@ | ||
#include "SlidingWindow.h" | ||
#include "Buffer.h" | ||
|
||
#include <assert.h> | ||
|
||
void TestDroppedLastMessage() | ||
{ | ||
WindowConfig conf; | ||
auto slidingWindow = SlidingWindow<int>(conf); | ||
for (int i = 1; i < 7; ++i) | ||
slidingWindow.Send(1, i); | ||
|
||
auto messageDump = slidingWindow.DumpMessages(1); | ||
assert(messageDump.size() == 5); | ||
int i = 1; | ||
for (auto msg : messageDump) | ||
{ | ||
assert(msg == i); | ||
++i; | ||
} | ||
} | ||
|
||
void TestMultipleClients() | ||
{ | ||
WindowConfig conf; | ||
conf.mMaxNumMessages = 8; | ||
auto slidingWindow = SlidingWindow<int>(conf); | ||
for (int i = 1; i < 7; ++i) | ||
slidingWindow.Send(1, i); | ||
|
||
for (int i = 1; i < 15; ++i) | ||
slidingWindow.Send(2, i); | ||
|
||
auto messageDump = slidingWindow.DumpMessages(1); | ||
assert(messageDump.size() == 6); | ||
int i = 1; | ||
for (auto msg : messageDump) | ||
{ | ||
assert(msg == i); | ||
++i; | ||
} | ||
|
||
messageDump = slidingWindow.DumpMessages(2); | ||
assert(messageDump.size() == 8); | ||
i = 1; | ||
for (auto msg : messageDump) | ||
{ | ||
assert(msg == i); | ||
++i; | ||
} | ||
} | ||
|
||
void TestDumpQuietClient() | ||
{ | ||
WindowConfig conf; | ||
auto slidingWindow = SlidingWindow<int>(conf); | ||
auto dump = slidingWindow.DumpMessages(1); | ||
assert(dump.size() == 0); | ||
} | ||
|
||
void TestNegativeNumMessagesInWindowConf() | ||
{ | ||
WindowConfig conf; | ||
conf.mMaxNumMessages = -5; | ||
bool failed = true; | ||
try | ||
{ | ||
auto slidingWindow = SlidingWindow<int>(conf); | ||
} | ||
catch (std::exception e) | ||
{ | ||
failed = false; | ||
} | ||
assert(!failed); | ||
} | ||
|
||
int main() | ||
{ | ||
TestDroppedLastMessage(); | ||
TestMultipleClients(); | ||
TestDumpQuietClient(); | ||
TestNegativeNumMessagesInWindowConf(); | ||
return 0; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Glad you didnt use time_t - you can also use chrono literals to write
= 10s