-
Notifications
You must be signed in to change notification settings - Fork 11
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
Cleanup #60
Cleanup #60
Conversation
… variables. This should be superceded in C++-17 with "optionals".
@trachten There are some compiler errors on your last commit here (869eccd). Also, do you use any compiler flags other than what is specified in |
Strange …. I’m just using llvm on a mac.
I’ve updated to fix the break (it compiles fine on my machines).
… On Jun 11, 2020, at 6:21 PM, Novak Boškov ***@***.***> wrote:
@trachten <https://github.com/trachten> There are some compiler errors <https://github.com/trachten/cpisync/runs/763207089> on your last commit here (869eccd <869eccd>).
Also, do you use any compiler flags other than what is specified in CmakeList.txt? Could you also share the compiler version you use with this project? I see you mention some compiler warnings in some of your commit messages and I'm just wondering if we use different compilers.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#60 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACM5LU6LCDSA7I57IXF6YJ3RWFKF7ANCNFSM4N2ZFMZA>.
---
Prof. Ari Trachtenberg ECE, Boston University
trachten@bu.edu http://people.bu.edu/trachten
|
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.
Just a few questions on try-catch
vs if
in Cuckoo
.
_commit_relocation_chain(relocStack); | ||
itemsCount++; | ||
return true; | ||
} | ||
} catch (overflow_error ignored) {} |
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.
This is within the cuckoo relocations loop.
In mostly full filters (say alpha>80
) this exception is going to be thrown a lot. Once per node in the cuckoo graph. That might make our insertions slower.
Is it because of ignored
that this will not be much slower? Is it because ignored
short-circuits the exception handler traversal?
The two try-catch
s above are different, they are called once per insertion.
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.
A good optimizer should optimize them out - see if you can do a quick profile to see whether it is really a problem?
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 tried it very naively:
int main() {
for (size_t ii=0; ii<4000; ii++) {
Cuckoo c = Cuckoo(7, 4, 1024, 500);
c.insert(DataObject(ZZ(Cuckoo::_rand(0, 1 << 12))));
}
}
And, well, Linux perf
shows that both if
and try-cache
(with GCC -O3
) spend about the same estimated number of cycles in insert
. Plain time
says the same...
No significant diference.
Great!
… On Jun 11, 2020, at 11:11 PM, 6/11/20, Novak Boškov ***@***.***> wrote:
@novakboskov commented on this pull request.
In src/Syncs/Cuckoo.cpp <#60 (comment)>:
> _commit_relocation_chain(relocStack);
itemsCount++;
return true;
- }
+ } catch (overflow_error ignored) {}
I tried it very naively:
int main() {
for (size_t ii=0; ii<4000; ii++) {
Cuckoo c = Cuckoo(7, 4, 1024, 500);
c.insert(DataObject(ZZ(Cuckoo::_rand(0, 1 << 12))));
}
}
And, well, Linux perf show both if and try-cache (with GCC -O3) spend about the same estimated number of cycles in insert. Plain time says the same.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#60 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACM5LU4RVV75I4IOECWIM33RWGMFJANCNFSM4N2ZFMZA>.
---
Ari Trachtenberg, Boston University
http://people.bu.edu/trachten mailto:trachten@bu.edu
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Random quote of the day:
A day without sunshine is like, night.
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
|
Some more commits.