Histogram tracer #323
Histogram tracer #323
Conversation
Codecov Report
@@ Coverage Diff @@
## master #323 +/- ##
=======================================
Coverage 99.77% 99.77%
=======================================
Files 27 28 +1
Lines 3932 3954 +22
=======================================
+ Hits 3923 3945 +22
Misses 9 9
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
|
||
| trace_stream << '\n'; | ||
| EXPECT_EQ(trace(add(0, 0)), R"( | ||
| --- # HISTOGRAM depth=0 |
There was a problem hiding this comment.
No. But this is yaml document start mark.
There was a problem hiding this comment.
The CSV can only have optional header line with column names. You cannot have any title line.
The fancier YAML wrapper would be
--- # HISTOGRAM
depth: 0
csv: |
opcode,count
ADD,1
PUSH1,2There was a problem hiding this comment.
So why not output YAML and then whoever wants CSV can do the conversion themselves (or in worst-case by skipping the top 3 lines) ?
There was a problem hiding this comment.
Because nobody wants YAML. I used --- because of no better ideas.
| }; | ||
| } // namespace | ||
|
|
||
| std::unique_ptr<Tracer> create_histogram_tracer(std::ostream& out) |
There was a problem hiding this comment.
Would add a comment (or on the class) explaining what this does, i.e. outputs a histogram of opcodes in a CSV format.
lib/evmone/tracing.cpp
Outdated
| m_out << "--- # HISTOGRAM depth=" << m_depth << "\nopcode,count\n"; | ||
| for (size_t i = 0; i < std::size(m_counts); ++i) | ||
| { | ||
| if (m_counts[i] != 0) |
There was a problem hiding this comment.
Should we have thre options?
- do not print zeroes
- one which also prints all the zero opcodes
- only those zero opcodes which are defined for the evm version
There was a problem hiding this comment.
Some of these options make sense, but I don't think it is worth doing these right now.
|
There's missing coverage on the |
chfast
force-pushed
the
histogram_tracer
branch
4 times, most recently
from
cb6d698 to
78a8475
Compare
lib/evmone/tracing.cpp
Outdated
| } | ||
| } | ||
|
|
||
| m_code = nullptr; |
There was a problem hiding this comment.
Maybe zero out m_counts and m_depth then, too?
There was a problem hiding this comment.
Well... because we have only single table in tracer we cannot measure internal calls separately. Therefore we should output only at depth=0 (and only then zeroing counters make sense). Moreover, reporting depth is useless.
There was a problem hiding this comment.
Wait, is on_execution_start called for each subcall? It changes m_code, how would it work after return from subcall then?
There was a problem hiding this comment.
Oh shit, it was suppose to be easy tracer example...
No description provided.