Skip to content

Added operation logging to JSON #1452

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from
Draft

Added operation logging to JSON #1452

wants to merge 9 commits into from

Conversation

pavelkryukov
Copy link
Member

@pavelkryukov pavelkryukov commented Feb 5, 2021

Rebased from #1451

Now when you want to log operations to JSON file - you need to add flag -j or --json-dump and logs will be saved to your current directory to file "logs.json".

andreyess and others added 3 commits February 5, 2021 22:46
Now when you want to log operations to JSON file - you need to add flag -j or --json-dump and logs will be saved to your current directory to file "logs.json".
@codecov
Copy link

codecov bot commented Feb 5, 2021

Codecov Report

Merging #1452 (ace7f64) into main (dfc125c) will decrease coverage by 0.46%.
The diff coverage is 63.04%.

❗ Current head ace7f64 differs from pull request most recent head 015a2f4. Consider uploading reports for the commit 015a2f4 to get more accurate results

@@            Coverage Diff             @@
##             main    #1452      +/-   ##
==========================================
- Coverage   99.78%   99.31%   -0.47%     
==========================================
  Files         141      141              
  Lines       11654    11542     -112     
==========================================
- Hits        11629    11463     -166     
- Misses         25       79      +54     
Impacted Files Coverage Δ
simulator/infra/ports/module.h 95.00% <ø> (-0.24%) ⬇️
simulator/modules/core/perf_sim.h 100.00% <ø> (ø)
simulator/infra/log.cpp 91.66% <50.00%> (-8.34%) ⬇️
simulator/modules/decode/decode.cpp 97.26% <50.00%> (-1.34%) ⬇️
simulator/modules/execute/execute.cpp 97.05% <50.00%> (-2.95%) ⬇️
simulator/modules/fetch/fetch.cpp 96.66% <50.00%> (-1.60%) ⬇️
simulator/modules/mem/mem.cpp 96.00% <50.00%> (-4.00%) ⬇️
simulator/modules/core/perf_sim.cpp 93.00% <53.33%> (-7.00%) ⬇️
simulator/func_sim/operation.h 96.42% <66.66%> (-2.88%) ⬇️
simulator/modules/writeback/writeback.cpp 97.36% <75.00%> (-2.64%) ⬇️
... and 54 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dfc125c...015a2f4. Read the comment docs.

@pavelkryukov pavelkryukov reopened this Feb 6, 2021
@@ -22,3 +22,7 @@ OStreamWrapper::~OStreamWrapper()
{
ostream.rdbuf( buffer);
}


std::ofstream* visualizer_logger = new std::ofstream(".\\logs.json", std::ofstream::out);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For testing, these variables should be non-static and reside in "Root" class. Objects can access them recursively.
Additionally, file name should be configurable from command line.

@@ -49,6 +50,9 @@ class Log
mutable LogOstream sout;
mutable LogOstream serr;

std::ofstream& jsonout();
bool jsonout_enabled = false;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By using LogOstream you may omit the boolean variable, and dump data unconditionally.

}

template<typename ISA>
void PerfSim<ISA>::stop_dump_json() {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It must be called in destructor, so file closing and proper finalization would be guaranteed.

@pavelkryukov
Copy link
Member Author

pavelkryukov commented Feb 7, 2021

@andreyess I ran these changes locally and an output JSON file was not finalized. I made few comments how to improve code and resolve that issue, if you have time you may take a look. Unit tests would be very useful to guarantee stable behavior.

@pavelkryukov pavelkryukov marked this pull request as draft December 11, 2021 11:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants