Miracle Grue .jsontoolpath stream reader #9
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| { | ||
| break; | ||
| } | ||
| end_char = buffer->sbumpc(); |
There was a problem hiding this comment.
narrowing conversion from std::basic_streambuf<char>::int_type (aka int) to signed type char is implementation-defined
| std::string CommandFileStream::getCommandLine() | ||
| { | ||
| std::string line; | ||
| if (! stream.is_open()) |
There was a problem hiding this comment.
statement should be inside braces
| if (! stream.is_open()) | |
| if (! stream.is_open()) { |
| { | ||
| std::string line; | ||
| if (! stream.is_open()) | ||
| return line; |
There was a problem hiding this comment.
statement should be inside braces
| return line; | |
| return line; | |
| } |
| if (last_char == CommandFileStream::K_CMD_CLOSE) | ||
| { | ||
| bracket_depth--; | ||
| if (bracket_depth <= 0) |
There was a problem hiding this comment.
statement should be inside braces
| if (bracket_depth <= 0) | |
| if (bracket_depth <= 0) { |
| { | ||
| bracket_depth--; | ||
| if (bracket_depth <= 0) | ||
| return line; |
There was a problem hiding this comment.
statement should be inside braces
| return line; | |
| return line; | |
| } |
| EXPECT_EQ(command_lines.size(), 7); | ||
| } | ||
|
|
||
| TEST(miracle_jtp_tests, command_stream) |
There was a problem hiding this comment.
avoid using "_" in test name "command_stream" according to Googletest FAQ
| } | ||
|
|
||
|
|
||
| TEST(miracle_jtp_tests, command_stream_vs_oneshot) |
There was a problem hiding this comment.
initializing non-owner argument of type testing::internal::TestFactoryBase * with a newly created gsl::owner<>
| } | ||
|
|
||
|
|
||
| TEST(miracle_jtp_tests, command_stream_vs_oneshot) |
There was a problem hiding this comment.
avoid using "_" in test case name "miracle_jtp_tests" according to Googletest FAQ
| } | ||
|
|
||
|
|
||
| TEST(miracle_jtp_tests, command_stream_vs_oneshot) |
There was a problem hiding this comment.
avoid using "_" in test name "command_stream_vs_oneshot" according to Googletest FAQ
|
|
||
| // check types | ||
| EXPECT_EQ(cmd_list_from_stream.size(), cmd_list_from_oneshot.size()); | ||
| for (size_t cmd_ii = 0; cmd_ii < cmd_list_from_stream.size(); cmd_ii++) |
There was a problem hiding this comment.
backward branch (for loop) is ID-dependent due to variable reference to cmd_list_from_stream and may cause performance degradation
Need to read .makerbot print command files.
The typical miracle grue .makerbot format contains an ascii text json file with a list of json dict entry for each command.
The currently used json reader does not have a streamable json parser, which means the entire json file has to be read into memory at once. For even moderately sized prints this can cause an out of memory crash. This stream is slower than directly one-shot reading the file, though not necessarily asymptotically slower. See comparison below. Note that one-shot reading is really not an option due to the size constraints. Minute long read times are rough to handle.
This command streamer is intended to parse a single command at a time, and does so by tracking {} depth. Alternatives:
`"test/data/icos_lava/print.jsontoolpath"
num commands: 98521
Sream read time: 1441ms
One-shot read time: 851ms
"shirleys_good_mask_cura_30oct_MMXL/print.jsontoolpath"
num commands: 2154012
Sream read time: 24332ms
One-shot read in: 19542ms
"magma-support1-absr-rr-nov3/print.jsontoolpath"
num commands: 4847882
Sream read time: 64591ms
One-shot read in: NA out of memory segfault`