cmd/internal/obj: delete Cputime and all its uses #19865
Comments
|
IIRC |
Debugvlog is pretty heavily overloaded. toolstash -cmp uses it to ask the compiler to emit much more detailed versions of the generated assembly, to better confirm that it is identical. Why does the linker need obj.Cputime? Any reason we couldn't use pprof instead? |
I believe it is just historical reason. |
|
At long time, we probably should get rid of obj.Cputime and use pprof. |
|
I guess the question is: Why not now? |
|
Slight misdiagnosis: It's not that variance in the timing causes toolstash -cmp failures. Rather, when toolstash -cmp fails, it causes the output to be about timing rather than about something useful. I'll send a note to golang-dev to see whether anyone is actively using this. |
|
Yeah, those timing lines are obnoxious when diffing toolstash log files. I for one wouldn't miss them. |
No problem with me, if you want to take it :) |
|
CL 39914 deletes all the assembler usages, which are the ones currently driving me crazy. If no one speaks up on behalf of the linker usages in the next few days, I'll do them too. |
|
CL https://golang.org/cl/39914 mentions this issue. |
Updates #19865 Change-Id: I24fbf5d79b5e4cac09c14cfff678a8215397b670 Reviewed-on: https://go-review.googlesource.com/39914 Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Updates golang#19865 Change-Id: I24fbf5d79b5e4cac09c14cfff678a8215397b670 Reviewed-on: https://go-review.googlesource.com/39914 Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
ALTree
added
NeedsFix
|
Change https://golang.org/cl/171730 mentions this issue: |
|
@josharian are you still interested in this CL? I can take a closer look and +2 if so. |
|
Yes please. |
|
Ok, SGTM. I am in transit at the moment, will take a closer look tomorrow. |
obj.Cputime is used to print elapsed time in various places in the assemblers. It is protected by ctxt.Debugvlog. However,
toolstash -cmpsets Debugvlog to true, causing rare spurious timing-based failures. They are rare because the elapsed time is basically always ~0. We could introduce a special flag to turn on/off this timing printing, but I'd rather just wipe it all out. We have good profiling and benchmarking tools; we should use them instead, if/when the assemblers ever become an actual performance bottleneck.Objections? Not sure who quite who to cc here for input, but maybe: @minux @cherrymui @randall77
The text was updated successfully, but these errors were encountered: