Refactor: simplify printXXX functions usages by apocelipes · Pull Request #625 · boyter/scc
Simplify printXXX functions usages. This PR makes code more simple and no harm to performance.
Why
Current usages of printXXX functions would check the Verbose/Trace/... flags twice (or 3 times for printXXXf functions). This is inefficient. printXXXf also calls printXXX, and all these functions cannot be inlined, that means the performance will be slowed down when we display some trace log (CI prints the trace log).
For code readability, the origin code has too much redundant code. There is also inappropriate coupling between functions.
For these reasons I did a refactoring of the whole printXXX function family.
Changes
In the first commit, I did:
- Simplify the printXXX functions' code by introduce
doPrintandprepareMsg.prepareMsgwill reuse its first paramater if we do not need call sprintf. Now these functions are no longer coupled. - This makes all printXXX functions can be inlined. So there is no performance impact even if we delete the outside if statements.
- Move printXXX functions to
trace.go. - Add tests for all these things.
In the second one:
- Since printXXX now can be inlined, the outside
if Verbose/Debugcondition is redundant. We already checked the flags in printXXX. The runtime logic doesn't change when these if statements are removed either:
// Origin code looks like: if Trace { // printTraceF("trace: %s", msg) inline into: if Trace { doPrint(os.Stdout, levelTrace, "trace: %s", msg) } } // After removing: // printTraceF("trace: %s", msg) inline into: if Trace { doPrint(os.Stdout, levelTrace, "trace: %s", msg) }
- Use
printXXXF("template %s", arg1, arg2...)instead ofprintXXX(fmt.Sprintf("template %s", arg1, arg2...)), the former is more simple and faster. printXXXfunctions usages changed: Dont put them intoif Verbose/Debug/Trace {...}blocks, call it directly:
// Before func A() { do some work if Verbose { printWarn("a warning message") } } // After func A() { do some work printWarn("a warning message") // won't print if the "Verbose" flag is false and no performance regressions }
I splitted the changes into 2 commits so that reviewers may easily do reviewing. Feel free to squash the commits or not if you are merging this PR.
