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 using java.lang.StringBuilder in the translation of String contatentation #9315

Closed
scabug opened this issue May 15, 2015 · 6 comments
Assignees
Milestone

Comments

@scabug
Copy link

scabug commented May 15, 2015

Even without considering indy-stringconcat, we could consider changing the desugaring of Scala’s String.+ to directly use Java’s StringBuilder, rather than collection.mutable.StringBuilder (which is just a wrapper for the former.)
This would save allocating the wrapper as well as the underlying builder, and, more importantly, present byte code more directly amenable to HotSpot’s StringBuilder optimizations.
If HotSpot could inline s.c.m.StringBuilder away, there is a chance those optimizations would kick in, but it seems we’re making things needlessly hard.
Does anyone know the history of our + desugaring?
@dragos Do you remember the motivation for switching to scala.StringBuilder in scala/scala@bee89ec ?
@scabug
Copy link
Author

scabug commented May 15, 2015

Imported From: https://issues.scala-lang.org/browse/SI-9315?orig=1
Reporter: @retronym

@scabug
Copy link
Author

scabug commented May 15, 2015

@retronym said:
Also, add an overload to scala.collection.mutable.StringBuilder to accept a CharSequence.

@scabug
Copy link
Author

scabug commented May 15, 2015

@dragos said:
@retronym I vaguely remember the time, but I am not 100% sure. I think someone wrote scala.StringBuilder in an attempt to have less dependencies on the JVM (and share more code with the .net backend). But I might be wrong. I think there were some concerns over how good or bad this implementation was, but in the end it seemed “safe”. I agree we should just go back to java.lang

@scabug
Copy link
Author

scabug commented Aug 12, 2015

@ijuma said:
Relevant email thread:

"However, it turns out there is an very important but underdocumented jvm optimization, OptimizeStringConcat, which is enabled by default. This optimization, which is pretty much only documented by its source code (http://hg.openjdk.java.net/jdk8/jdk8/hotspot/file/87ee5ee27509/src/share/vm/opto/stringopts.cpp), is capable of turning something of approximately the form new StringBuilder().append(x).append(y)....append(z).toString() (aka about what javac would generate if you use x + y + ... + z) into a single char[] allocation of the optimal length, which is a big performance boost over what the normal java code does (2 char[] allocations at the absolute minimum if the initial size is correct, normally more)
The issue is that this same optimization doesn't work for the identical scala code."

https://groups.google.com/d/msg/scala-language/AN2Ymk1J1ik/Tpq9vpQCIQAJ

@scabug
Copy link
Author

scabug commented Sep 9, 2015

@soc said:
scala/scala#4737

@scabug scabug closed this as completed Feb 3, 2016
@scabug
Copy link
Author

scabug commented Feb 3, 2016

@ijuma said:
Nice work. JEP 280: Indify String Concatenation is implemented in Java 9 and is related:

http://openjdk.java.net/jeps/280

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

2 participants