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

Consider introducing special cases for TypeRefs of small arity #7186

Open
scabug opened this issue Feb 26, 2013 · 12 comments
Open

Consider introducing special cases for TypeRefs of small arity #7186

scabug opened this issue Feb 26, 2013 · 12 comments

Comments

@scabug
Copy link

scabug commented Feb 26, 2013

Jason proposed we would look into special casing TypeRefs that have small arity (probably 0, 1 and 2) in order to reduce memory consumption coming from references to :: objects.

@scabug
Copy link
Author

scabug commented Feb 26, 2013

@scabug
Copy link
Author

scabug commented Mar 27, 2013

@retronym said:
The distribution of created TypeRefs when compiling scala/collection/**

{noformat}

TypeRef args distribution

0 1895430 47.26%
1 867559 21.63%
2 887096 22.12%
3 357780 8.92%
4 1287 0.03%
5 1156 0.03%
6 69 0.00%
7 8 0.00%
8 8 0.00%
9 10 0.00%
{code}

I've prototyped a solution that avoids the :: for one-arg type refs. The tradeoff is slower pattern matching on TypeRefs, which now must go through an unapply method. I couldn't measure any different in compiler speed.

I also noticed that some TypeRef subclasses had a needless outer pointer, and have eliminated that by unnesting them out of object TypeRef. For fun, I added infrastructure to the Instrumented tests to show the JVM object size.

https://github.com/retronym/scala/compare/ticket/7186

I'll extract the changes to slim down TypeRef to another branch and submit a PR, as they are a clear micro-improvement. But I'll leave the first change on the branch; @grzegorz, perhaps you can suggest a scientific way to benchmark the memory impact? I was using YourKit manually, but I didn't have good control over when I took the memory snapshot.

@scabug
Copy link
Author

scabug commented Mar 27, 2013

@magarciaEPFL said:
It's a bit off topic, but what's the reason that "redundant outer pointers" aren't removed by, say, the optimizer? Does "separate compilation" stand in the way?

At least for "late closure classes", the new optimizer does a thorough job of squashing unneeded outer pointers,
magarciaEPFL/scala@a4bed20

I thought that Constructors would detect (most) cases of redundant outer pointers, isn't that enough?

@scabug
Copy link
Author

scabug commented Mar 27, 2013

@retronym said:
That's a good question. I'm not really sure what is triggering the creation and retention of that outer pointer.

e.g. what is different about Types/TypeRef to this example?

scala> trait T { val t = "t"; class C { def tt = t }; object O { def foo = new C {} } }
defined trait T

scala> new T{}.O.foo.getClass.getDeclaredFields
res2: Array[java.lang.reflect.Field] = Array()

Needs some investigation.

@scabug
Copy link
Author

scabug commented Mar 27, 2013

@magarciaEPFL said:
I'll take a look what can be done (if all else fails, via the new optimizer).

@scabug
Copy link
Author

scabug commented Mar 27, 2013

@retronym said:
I've added a recent optimization of Paul's to the branch.

scala/scala@79b18ccf9

That takes TypeRefs from 54 bytes to 40 bytes, by moving usually-null fields to global maps. Back-of-the-envelope-wise, it should save about 2-3% of our retained memory.

@scabug
Copy link
Author

scabug commented Mar 27, 2013

@retronym said:
The non-controversial change to unnest anonymous TypeRef subclasses from object TypeRef has been merged scala/scala#2325.

But it would still be great to pin down the real raison d'être for that extra outer field.

@scabug
Copy link
Author

scabug commented Jul 10, 2013

@adriaanm said:
Unassigning as milestone deadline was reached.

@scabug
Copy link
Author

scabug commented Feb 10, 2014

@adriaanm said:
Since 2.11.0-RC1 is one week away, pushing all non-blockers without PR to 2.11.1-RC1. Please undo the change if I missed work in progress.

@scabug
Copy link
Author

scabug commented Aug 5, 2014

@gkossakowski said:
The 2.11.2 is out so I'm rescheduling the issue for 2.11.3.

@scabug
Copy link
Author

scabug commented Nov 4, 2014

@retronym said:
Updating fix-by version to 2.11.5.

@scabug
Copy link
Author

scabug commented Apr 4, 2017

@SethTisue said:
(pinging Jason in case this isn't already on his mind in the current round of compiler performance work)

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

3 participants