Eliminate the use of asInstanceOf in Eval.

[TomasMikula]

No description provided.

[codecov-io]

Current coverage is 88.98%

Merging #924 into master will increase coverage by +0.04% as of 3fc1faa

@@            master    #924   diff @@
======================================
  Files          179     179       
  Stmts         2125    2124     -1
  Branches        42      42       
  Methods          0       0       
======================================
  Hit           1890    1890       
  Partial          0       0       
+ Missed         235     234     -1

Review entire Coverage Diff as of 3fc1faa

Powered by Codecov. Updated on successful CI builds.

[ceedubs]

@non If I recall, you put some effort into optimizing this. Any thoughts?

[non]

Hey @TomasMikula! Sorry this has been hanging out so long.

Do you have a sense of the relative performance before and after? I'll try to look at some benchmarks myself, but there could be a difference. The change to allow tailrec is interesting -- I'm not sure if the overall effect will be better or worse.

Let me know if you have some results to share, or if you are going to work on this. I'll try to get to it when I have some time to look into it (I may need similar benchmarks to continue working on #989).

[TomasMikula]

I haven't done any benchmarking. My primary goal was to get rid of asInstanceOfs and uses of Any. It's hard to tell how it will perform relative to the original code. There seems to be one more allocation per iteration of re-associating to the right in my code. On the other hand, it avoids those asInstanceOfs (not sure how costly they are at runtime).

[non]

OK thanks. I'll try to do some tests and comment here when I know more.

[non]

I haven't done the benchmarking and don't anticipate doing it anytime soon.

@TomasMikula I feel bad having your PR sitting here, and don't expect it to be merged soon. Would you mind if I closed it? I could open an issue encouraging us to investigate whether the casts are needed.

[TomasMikula]

No need to feel bad. I didn't anticipate there would be a performance concern when I opened the PR.

[TomasMikula]

scala/scala#6065 would make it possible to eliminate those asInstanceOf calls without introducing extra allocations.