Use of reflect.TypeOf slows down Go execution by 33%

[davesisson]

A coworker of mine found that replacing if reflect.TypeOf with ctx.(IParameterListContext) in the generated grammar improved the performance noticeably. I suggest we remove the use of reflect.TypeOf as much as possible in the Go implementation.

[davesisson]

Sample Before:

  var t = s.GetTypedRuleContext(
    reflect.TypeOf((*IMyTypeContext)(nil)).Elem(), i)

Sample After:

  var t antlr.RuleContext; j := 0
  for _, ctx := range s.GetChildren() {
    if _, ok := ctx.(IMyTypeContext); ok {
      if j==i { t = ctx.(antlr.RuleContext); break } else { j++ } 
    }
  }

[wdscxsj]

@millergarym @pboyer Care to take a look? Thanks.

[pboyer]

Good find! I'll try and take a look at this soon.

…

On Mon, Apr 23, 2018, 10:55 PM Yang Yang ***@***.***> wrote: @millergarym <https://github.com/millergarym> @pboyer <https://github.com/pboyer> Care to take a look? Thanks. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2278 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA37eT15RH77LMb2mJ6qWfDtNQh6qMEYks5trpQLgaJpZM4TeKd1> .

[millergarym]

Cool.

@pboyer I setup up a regression test environment when last doing performance work. It might be of interest. see https://github.com/wxio/antlr4-jenkins/

@davesisson any chance of getting an example grammar and input. Benchmarking can be quite context dependant. Also, imo its nicer to have a really problem that someone cares about than a substituent.

[davesisson]

I agree that a real problem would be useful. Unfortunately the grammar and input i have are all internal (but have no fear that it's complex :) ). If I had to choose a suitable replacement I'd choose a fully featured language like in the antlr grammars collection. It's bigger than the C grammar so how about Java 9? https://github.com/antlr/grammars-v4/blob/master/java9/Java9.g4

…

On Tue, Apr 24, 2018 at 10:30 PM Gary Miller ***@***.***> wrote: Cool. @pboyer <https://github.com/pboyer> I setup up a regression test environment when last doing performance work. It might be of interest. see https://github.com/wxio/antlr4-jenkins/ @davesisson <https://github.com/davesisson> any chance of getting an example grammar and input. Benchmarking can be quite context dependant. Also, imo its nicer to have a really problem that someone cares about than a substituent. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2278 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ALUC_ShEgDAmLbtJksVVwuwcv9wsl1MLks5tsAoEgaJpZM4TeKd1> .

[wdscxsj]

Any news, please? In my case, removing reflection from the parser does make it faster, but not by much (25.60s -> 24.59s, internal project). Still noticeably slower than the Java version.

I modified antlr.jar/org/antlr/v4/tool/templates/codegen/Go/Go.stg according to the code samples by @davesisson. Should I make a PR or wait for something good to happen? Of course, all credits to @davesisson and his coworker.

[KvanTTT]

but not by much (25.60s -> 24.59s, internal project)

It's quite a good improvement (4%).

Should I make a PR or wait for something good to happen?

Yes, please do it if you can. In upcoming 4.9.3 we also have other Go runtime performance improvements: #3243