New compiler parser benchmarks and enabling by default?

Matthew Dempsky

unread,

Sep 9, 2016, 9:20:46 PM9/9/16

to golang-dev

Yesterday I benchmarked the compiler with and without newparser. Initial benchstat results here: https://docs.google.com/a/google.com/document/d/1QHVzyzNqWQGAwrz0uOYMqmzyHUESBDZZS-Ezaf_ME1Y/pub

For the most part, the timing differences appear to be within noise, but there is a statistically significant slowdown in 24% of packages (and speedup in 6%).

What are people's impressions of these numbers? Are these good enough to enable the new parser by default so it can get more testing?

Also FYI, my focus so far has been primarily on correctness/compatibility. I expect there's room for performance improvements still and intend to investigate that further before proposing to remove the old parser.

Brad Fitzpatrick

unread,

Sep 9, 2016, 9:22:14 PM9/9/16

to Matthew Dempsky, golang-dev

You need permission to access this published document.

You are signed in as brad...@golang.org, but you don't have permission to access this published document. You may need to sign in as a different user.

--
You received this message because you are subscribed to the Google Groups "golang-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Matthew Dempsky

unread,

Sep 9, 2016, 9:25:14 PM9/9/16

to Brad Fitzpatrick, golang-dev

https://docs.google.com/document/d/1isk4grKz4TjjtNXaPKuzLOE-b4xONON3GGURetF0diU/pub

(╯°□°）╯︵ ┻━┻

Robert Griesemer

unread,

Sep 9, 2016, 10:08:07 PM9/9/16

to Matthew Dempsky, Brad Fitzpatrick, golang-dev

I'd go forward with it, especially because we still have the option to revert for some time (before we rip out the old parser). It's the best way to find problems. If people complain or too many issues show up, we flip the flag back.

It might be interesting to see what's special about the packages which show the largest time increases. But this investigation can happen after the switch.

But I'm happy to hear other opinions.

- gri

PS: If the decision is to move ahead, I suggest to wait with flipping the switch until Mon morning...

Ian Lance Taylor

unread,

Sep 9, 2016, 10:23:58 PM9/9/16

to Matthew Dempsky, golang-dev

On Fri, Sep 9, 2016 at 12:20 PM, 'Matthew Dempsky' via golang-dev
<golan...@googlegroups.com> wrote:

Some of those slowdowns are so dramatic that it seems that it ought to
be possible to find out why they are happening.

After all the work to speed up the compiler, and facing the fact that
it is still slow, I'm reluctant to endorse slowing it down further,
even if not by very much.

Ian

Austin Clements

unread,

Sep 9, 2016, 10:29:19 PM9/9/16

to Matthew Dempsky, golang-dev

On Fri, Sep 9, 2016 at 3:20 PM, 'Matthew Dempsky' via golang-dev <golan...@googlegroups.com> wrote:

Also FYI, my focus so far has been primarily on correctness/compatibility. I expect there's room for performance improvements still and intend to investigate that further before proposing to remove the old parser.

Perhaps it would be worth putting a few days effort into speeding things up before flipping over. I bet there's some low-hanging fruit here.

Rob Pike

unread,

Sep 9, 2016, 11:45:54 PM9/9/16

to Austin Clements, Matthew Dempsky, golang-dev

I thought this parser was supposed to speed things up. If I read the numbers right and those bad slowdowns are for a compilation, which includes code generation as well as parsing, there is a bad performance bug that should be fixed before turning this on.

Also, what happened to the promise of a faster parser?

-rob

Matthew Dempsky

unread,

Sep 10, 2016, 12:20:27 AM9/10/16

to Rob Pike, Austin Clements, golang-dev

Background: the new parser also brings a new AST, more similar to go/ast's. But because most of the compiler mid-layer works on the gc.Node tree instead, there's currently an extra step to transform the new AST into the old AST. Long term we expect to transition the compiler to exclusively using the new AST and that transformation step will go away.

It's not too surprising to me that the new-to-old AST transformation step would cause some slowdown in the short-term, which we'll recover once the transition is complete.

But everyone's performance concerns are noted. I'll investigate what can be done to make the transition period more performance neutral and/or increase confidence that the ultimate payoff will be worthwhile.

Robert Griesemer

unread,

Sep 10, 2016, 12:43:15 AM9/10/16

to Matthew Dempsky, Rob Pike, Austin Clements, golang-dev

FWIW, currently this parser is also not running concurrently, due to temporary adjustments to make it 100% compatible with the current tests (line numbers, etc.). Once these are resolved, the new parser will be able to parse package files concurrently.

I suspect some of the slowdown may be due to extra GC work as the compiler now allocates a 2nd AST (which is thrown out quickly).

- gri

Reply all

Reply to author

Forward