Intrinsfy raw and s String interpolators #6093

retronym · 2017-09-22T05:33:39Z

Boxing and varargs incur a significant penalty for string
interpolation, as explored in:

https://medium.com/@dkomanov/scala-string-interpolation-performance-21dc85e83afd

This pull request takes an initial, big step back towards decent performance
by translating calls to these interpolators to string concatenation, which
itself is already translated to string builder appends in the JVM backend.

I have also tried to be a little smarter with the way we size the string
builder.

Longer term, we should build atop of JEP-280 to extract even better
performance with less code for us to maintain. But that's not available
until Java 9.

lrytz · 2017-09-23T08:47:42Z

JavaUniverseForce.scala must be updated.
===========================================================
 DIFF:
===========================================================
--- a
+++ b
@@ -321,2 +321,3 @@
     definitions.StringContextClass
+    definitions.StringContextModule
     definitions.QuasiquoteClass

Also the following, which I cannot pin down by just looking at the jenkins log:

[error] - junit/test
[error]   - junit/test:compileIncremental failed: scala.StringContext$InvalidEscapeException: invalid escape '\x' not one of [\b, \t, \n, \f, \r, \\, \", \'] at index 0 in "\x". Use \\ for literal \.

retronym · 2017-09-25T22:55:31Z

Fixed up the test failure and refactored the implementation some.

I'm happy with the 2.12.5 milestone for this one, seems too risky to rush in to 2.12.4.

dwijnand · 2017-09-29T07:54:26Z

src/reflect/scala/reflect/internal/StdNames.scala

    val experimental: NameType         = "experimental"
    val f: NameType                    = "f"
+    val s: NameType                    = "s"
+    val raw_ : NameType                = "raw"


alphabetical order?

no strong opinion, but it seems the others here are alphabetical indeed.

retronym · 2018-01-15T04:28:52Z

@lrytz Are you happy with the final state of this to target 2.12.5?

lrytz

Looks good, only a few cosmetic cleanups.

lrytz · 2018-01-22T09:56:20Z

src/reflect/scala/reflect/internal/Definitions.scala

      lazy val StringContext_f = getMemberMethod(StringContextClass, nme.f)
+      lazy val StringContext_s = getMemberMethod(StringContextClass, nme.s)
+      lazy val StringContext_raw = getMemberMethod(StringContextClass, nme.raw_)
+      lazy val StringContext_constructor = getMemberMethod(StringContextClass, nme.CONSTRUCTOR)


this is unused

lrytz · 2018-01-22T09:58:04Z

src/reflect/scala/reflect/internal/StdNames.scala

    val experimental: NameType         = "experimental"
    val f: NameType                    = "f"
+    val s: NameType                    = "s"
+    val raw_ : NameType                = "raw"


no strong opinion, but it seems the others here are alphabetical indeed.

lrytz · 2018-01-22T09:58:20Z

src/reflect/scala/reflect/internal/TreeGen.scala


  def mkLiteralUnit: Literal = Literal(Constant(()))
  def mkUnitBlock(expr: Tree): Block = Block(List(expr), mkLiteralUnit)
+  def mkLiteralString(s: String): Literal = Literal(Constant(s)).setType(StringTpe)


lrytz · 2018-01-22T10:06:20Z

src/compiler/scala/tools/nsc/typechecker/RefChecks.scala

    }
+
+    private object StringContextIntrinsic {
+      def unapply(t: Apply): Option[(List[Tree], List[Tree])] = {


Could return an Option[(List[Literal], List[Tree])] to avoid the casts at the use site, though it would require another map in the raw case. Up to you, both ways is fine.

lrytz · 2018-01-22T10:08:33Z

src/compiler/scala/tools/nsc/typechecker/RefChecks.scala

          transform(qual)
-
+      case StringContextIntrinsic(treated, args) =>
+        var tree: Tree = treated.head


maybe call it result to avoid shadowing (and confusion)

lrytz

pushed my review suggestions

lrytz · 2018-02-23T14:23:09Z

/synch

retronym · 2018-03-05T02:28:01Z

pushed my review suggestions

Thanks!

SethTisue · 2018-03-15T14:56:24Z

questions: this is intrinsics and not a macro in order to get it into 2.12.x, because bincompat...? and the motivation for getting it into 2.12.x instead of just waiting for 2.13.x is compiler performance...?

retronym · 2018-03-15T22:22:34Z

Yes, bin compat rules out the macro in 2.12. Source compat, taken strictly, also makes it had to turn a regular method into a macro in a major release, but we still plan to do that in 2.13: https://github.com/scala/scala-dev/issues/475 The compiler already avoided this iniefficiency in really hot code paths, we didn’t expect nor measure a discernible important time improvement after this change. But we expected expect application code to benefit (example where it would have helped: sbt/util#152), hence the desire to deliver earlier than 2.13

SethTisue · 2018-03-15T23:10:20Z

Nice, forgot about 475! Thx /cc @hrhino

xuwei-k · 2018-05-09T09:44:05Z

scala/bug#10870 😢

eed3si9n · 2018-10-09T23:34:47Z

scala/bug#11196

scala-jenkins added this to the 2.12.5 milestone Sep 22, 2017

retronym force-pushed the topic/string-s branch from 4dd601f to 49e4a08 Compare September 25, 2017 22:54

adriaanm mentioned this pull request Sep 27, 2017

String Interpolation by macros #6041

Closed

dwijnand reviewed Sep 29, 2017

View reviewed changes

smarter mentioned this pull request Oct 11, 2017

Optimize the raw"" and s"" string interpolators scala/scala3#3308

Closed

mkeskells mentioned this pull request Oct 12, 2017

StringInterpolation is slow and inefficient rorygraves/scalac_perf#24

Open

retronym force-pushed the topic/string-s branch from be070a8 to a96e4e2 Compare December 1, 2017 04:43

retronym changed the title ~~WIP Intrinsfy raw and s interpolators~~ Intrinsfy raw and s String interpolators Dec 1, 2017

hrhino mentioned this pull request Jan 9, 2018

Add the sm"" interpolator to the standard library. #6254

Closed

1 task

retronym requested a review from lrytz January 15, 2018 04:28

lrytz requested changes Jan 22, 2018

View reviewed changes

Intrinsify StringConcat.{s,raw} and improve string concat code gen

72642f6

lrytz added the release-notes worth highlighting in next release notes label Feb 23, 2018

lrytz force-pushed the topic/string-s branch from a96e4e2 to 72642f6 Compare February 23, 2018 12:34

lrytz self-assigned this Feb 23, 2018

lrytz approved these changes Feb 23, 2018

View reviewed changes

lrytz merged commit 9fc1356 into scala:2.12.x Feb 23, 2018

dwijnand mentioned this pull request Feb 23, 2018

Performance optimizations around logging sbt/util#152

Merged

eed3si9n mentioned this pull request Mar 4, 2018

Deprecate string-building using + with a non-String type on the left (aka any2stringadd) #6315

Merged

SethTisue mentioned this pull request Mar 13, 2018

document relationship to interpolation changes in Scala 2.12.5 outr/perfolation#1

Closed

plokhotnyuk mentioned this pull request Mar 14, 2018

Project Merging? plokhotnyuk/fast-string-interpolator#3

Closed

lrytz added 2.12 and removed 2.12 labels Mar 14, 2018

dwijnand mentioned this pull request Apr 25, 2018

Update check file to fix the build #6557

Merged

xuwei-k mentioned this pull request May 9, 2018

large string interpolation cause StackOverflowError since Scala 2.12.5 scala/bug#10870

Closed

dwijnand mentioned this pull request Jun 12, 2018

Adds AnyTap and AnyPipe #6767

Closed

mkeskells mentioned this pull request Jul 21, 2018

[EXPERIMENT] Instringsic stripMargin #6957

Closed

exoego mentioned this pull request Sep 11, 2018

Recommend string interpolation instead of toString #7201

Merged

eed3si9n mentioned this pull request Oct 9, 2018

s String interpolator takes backslash literally when used with by-name parameter scala/bug#11196

Closed

nrinaudo mentioned this pull request Jan 3, 2019

Flesh out the article on string concatenation nrinaudo/scala-best-practices#11

Closed

retronym mentioned this pull request Jan 16, 2019

Make StringContext.{f,raw} into macros in 2.13 scala/bug#11366

Closed

2 tasks

renovate bot mentioned this pull request Jun 27, 2021

chore(deps): update dependency scala to v2.12.14 w0rxbend/kzonix-opencv-app#6

Merged

1 task

renovate bot mentioned this pull request Aug 11, 2021

Update dependency scala to v2.12.14 - autoclosed bgoonz/The-Algorithms#7

Closed

1 task

Intrinsfy raw and s String interpolators #6093

Intrinsfy raw and s String interpolators #6093

Uh oh!

Conversation

retronym commented Sep 22, 2017

Uh oh!

lrytz commented Sep 23, 2017

Uh oh!

retronym commented Sep 25, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dwijnand Sep 29, 2017

Choose a reason for hiding this comment

Uh oh!

lrytz Jan 22, 2018

Choose a reason for hiding this comment

Uh oh!

retronym commented Jan 15, 2018

Uh oh!

lrytz left a comment

Choose a reason for hiding this comment

Uh oh!

lrytz Jan 22, 2018

Choose a reason for hiding this comment

Uh oh!

lrytz Jan 22, 2018

Choose a reason for hiding this comment

Uh oh!

lrytz Jan 22, 2018

Choose a reason for hiding this comment

Uh oh!

lrytz Jan 22, 2018

Choose a reason for hiding this comment

Uh oh!

lrytz Jan 22, 2018

Choose a reason for hiding this comment

Uh oh!

lrytz left a comment

Choose a reason for hiding this comment

Uh oh!

lrytz commented Feb 23, 2018

Uh oh!

retronym commented Mar 5, 2018

Uh oh!

SethTisue commented Mar 15, 2018

Uh oh!

retronym commented Mar 15, 2018 via email

Uh oh!

SethTisue commented Mar 15, 2018

Uh oh!

xuwei-k commented May 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eed3si9n commented Oct 9, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

retronym commented Sep 25, 2017 •

edited

Loading

xuwei-k commented May 9, 2018 •

edited

Loading