-
Notifications
You must be signed in to change notification settings - Fork 38
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
bench: add FizzyCEngine #561
Conversation
ea47ca0
to
356c5ee
Compare
6871e47
to
c517410
Compare
Codecov Report
@@ Coverage Diff @@
## master #561 +/- ##
==========================================
+ Coverage 98.24% 98.25% +0.01%
==========================================
Files 63 64 +1
Lines 9370 9425 +55
==========================================
+ Hits 9206 9261 +55
Misses 164 164 |
dfdbfbf
to
6525b9d
Compare
ded8136
to
17f123a
Compare
79b8a72
to
6a190e6
Compare
b6eea83
to
0a80dfc
Compare
085143c
to
f3ab3df
Compare
0a80dfc
to
486219a
Compare
bed7b06
to
7bf49d1
Compare
66c71d0
to
9791ba5
Compare
b7af6f9
to
34bcd69
Compare
test/utils/fizzy_c_engine.cpp
Outdated
if (!fizzy_find_exported_function( | ||
fizzy_get_instance_module(m_instance.get()), std::string{name}.c_str(), &func_idx)) | ||
{ | ||
// TODO check function type |
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.
Do we have this solved by the API?
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.
No, it's the same as in C++ API.
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.
fizzy_engine.cpp
does check the signature.
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.
But i think we can ignore that because we decided to do it only once in fizzy_engine and ignore the other implementaitons.
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 thought to reuse the code from fizzy_engine,cpp
, shoud I just remove TODO?
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.
Yeah, I think it is enough having it in one engine.
d32ba10
to
0b71661
Compare
Comparing fizzy vs fizzyc
|
This is the only benchmark which should be affected and 6% does not seem to be terrible, but wonder if it can be improved or if this is just noise. Anyway we should handle it outside of this PR. |
50db6d6
to
102f3e3
Compare
test/utils/fizzy_c_engine.cpp
Outdated
@@ -0,0 +1,121 @@ | |||
// Fizzy: A fast WebAssembly interpreter | |||
// Copyright 2019-2020 The Fizzy Authors. |
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.
// Copyright 2019-2020 The Fizzy Authors. | |
// Copyright 2020 The Fizzy Authors. |
No description provided.