go/printer: measure lines/construct in generated output rather than incoming source
No change to $GOROOT/src, misc formatting.
Nice side-effect: almost 3% faster runs because it's much faster to compute
line number differences in the generated output than the incoming source.
Benchmark run, best of 5 runs, before and after:
BenchmarkPrint 200 12347587 ns/op
BenchmarkPrint 200 11999061 ns/op
Fixes issue 4504.
LGTM Seems fine, but I'm not an expert on this code. https://codereview.appspot.com/69260045/diff/160001/src/pkg/go/printer/nodes.go File src/pkg/go/printer/nodes.go (right): ...
10 years, 6 months ago
(2014-02-27 18:53:14 UTC)
#2
PTAL https://codereview.appspot.com/69260045/diff/160001/src/pkg/go/printer/nodes.go File src/pkg/go/printer/nodes.go (right): https://codereview.appspot.com/69260045/diff/160001/src/pkg/go/printer/nodes.go#newcode923 src/pkg/go/printer/nodes.go:923: if _, ok := s.(*ast.LabeledStmt); ok { On ...
10 years, 6 months ago
(2014-02-27 19:30:30 UTC)
#3
PTAL
https://codereview.appspot.com/69260045/diff/160001/src/pkg/go/printer/nodes.go
File src/pkg/go/printer/nodes.go (right):
https://codereview.appspot.com/69260045/diff/160001/src/pkg/go/printer/nodes....
src/pkg/go/printer/nodes.go:923: if _, ok := s.(*ast.LabeledStmt); ok {
On 2014/02/27 18:53:14, adonovan wrote:
> Does this work for (perverse) nested label stmts, e.g. L1: L2: for{}?
good catch! fixed and updated tests
https://codereview.appspot.com/69260045/diff/160001/src/pkg/go/printer/testda...
File src/pkg/go/printer/testdata/declarations.golden (right):
https://codereview.appspot.com/69260045/diff/160001/src/pkg/go/printer/testda...
src/pkg/go/printer/testdata/declarations.golden:411: x1 = X{} // foo
On 2014/02/27 18:53:14, adonovan wrote:
> Why are there multiple spaces here?
>
> I would expect just one, plus padding of shorter identifiers up to the longest
> identifier---not applicable here since len("x1")==len("x2").
Actually, these are tabs for separation in the actual .golden file, and there's
exactly one tab (which is what is expected), but codereview doesn't show tabs.
The .golden files show the printer output using tabs for alignment rather than
spaces. I should probably change that to match gofmt better (another CL). The
gofmt'ed version looks correct (manually verified).
*** Submitted as https://code.google.com/p/go/source/detail?r=946dbec17445 *** go/printer: measure lines/construct in generated output rather than incoming source ...
10 years, 6 months ago
(2014-02-27 19:36:02 UTC)
#5
*** Submitted as https://code.google.com/p/go/source/detail?r=946dbec17445 ***
go/printer: measure lines/construct in generated output rather than incoming
source
No change to $GOROOT/src, misc formatting.
Nice side-effect: almost 3% faster runs because it's much faster to compute
line number differences in the generated output than the incoming source.
Benchmark run, best of 5 runs, before and after:
BenchmarkPrint 200 12347587 ns/op
BenchmarkPrint 200 11999061 ns/op
Fixes issue 4504.
LGTM=adonovan
R=golang-codereviews, adonovan
CC=golang-codereviews
https://codereview.appspot.com/69260045
Issue 69260045: code review 69260045: go/printer: measure lines/construct in generated output...
(Closed)
Created 10 years, 6 months ago by gri
Modified 10 years, 6 months ago
Reviewers: gobot
Base URL:
Comments: 4