Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

special apply methods #42

Open
mkeskells opened this issue Jan 25, 2018 · 7 comments
Open

special apply methods #42

mkeskells opened this issue Jan 25, 2018 · 7 comments

Comments

@mkeskells
Copy link
Collaborator

mkeskells commented Jan 25, 2018

the compiler handles List.apply() specially (with 0 args)

consider and discuss with @retronym i fthis is valid for other cases

e.g list.apply with explicit args

Seq, Set, Map for 0 args

@hrhino
Copy link
Collaborator

hrhino commented Jan 25, 2018

Note that Array(x, y) is already optimized to { val tmp = new Array(2); tmp(0) = x; tmp(1) = y; tmp }

@rorygraves
Copy link
Owner

Steps:

  1. How often to these cases occur
  2. What cases are worth optimising
  3. Apply changes as appropriate.

@mkeskells
Copy link
Collaborator Author

appropriate is based on safety, bytecode size and benchmarking

@retronym
Copy link

For reference, here's a WIP compiler rewrite that unrolls small List(a, b, c .. ) into a :: b :: .. :: Nil

The resulting bytecode would be cleaner with runtime support, rewrite to ScalaRuntime.list_apply_5(a, b, c, d, e), as suggested by this ticket.

Just adding in actual overloads of List.apply is a bit problematic because overload can interfere with type inference.

@retronym
Copy link

Also worth noting that not all of the innefficiency of List.apply at the moment comes from the packing the elements into varargs array. The generic result building code used internally will be replaced by a specialized version for List in 2.13.x.

@mkeskells
Copy link
Collaborator Author

mkeskells commented Mar 15, 2018

So we look at this for the 2.13 branch primarily - labelled as such.

We also cant add to the List.apply because we have no way to disambiguate List.apply _
I had a email chain from a few years ago, predating the List() rewrite I think

@mkeskells
Copy link
Collaborator Author

mkeskells commented Mar 15, 2018

Hi @retronym Just noticed the date of the 'WIP compiler rewrite' Nov 3 2013 - is that when we had the List() discussion?

How many other nuggets of stalled work do you have hidden away needing some affection from us
😱

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants