-
Notifications
You must be signed in to change notification settings - Fork 14
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
Fixes #16 #17
Fixes #16 #17
Conversation
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.
I had just one suggestion that might be totally unnecessary. Other than that it looks good! 👍
@@ -36,7 +36,8 @@ public FilterRule(String input, Map configuration) throws ParsingException { | |||
|
|||
@Override | |||
public boolean consume(BulletRecord record) { | |||
if (!specification.isAcceptingData() || !specification.filter(record)) { | |||
// If rule is expired, not accepting data or does not match filters, don't consume... | |||
if (isExpired() || !specification.isAcceptingData() || !specification.filter(record)) { |
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.
It might be over-optimizing, but you could add a boolean member variable to AbstractRule, maybe called "expired", and check this first in isExpired() before doing the system call. I think the 'System.currentTimeMillis()' system call is going to be an order of magnitude slower than checking a member variable. So you could avoid the System call for all the times isExpired() is called after the rule has expired but before the next tick occurs...
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.
Thought about adding that but since a query generally runs longer than a tick, the number of the times we would check this boolean (and do the time check) while the query is running would be >> the number of times we don't do the time check after the query has expired and before the tick.
No description provided.