New setting `-opt-inline-from` to control where to inline from #5964

lrytz · 2017-06-27T16:10:21Z

Deprecates -opt:l:classpath and -opt:l:project, introduces -opt:l:inline which enables cross-method optimizations. The inliner will only inline code from classes listed in -opt-inline-from, which is empty by default. So specifying only -opt:l:inline actually doesn't enable any inlining.

Proposed syntax for -opt-inline-from:

slices of qualified class names: List, scala.collection.List, collection.mutable, etc
*: longest match of identifier parts (not .)
**: longest match of anything
<sources>: allows inlining from classes being compiled in the current compilation run (implemented in a follow-up PR -opt-inline-from:<sources> to allow inlining from compilation units #5989)
leading ! to make a rule excluding

Examples


`*`	classes in the empty package
`scala.*`	classes in package scala, including the package object (`scala.package$`)
`**`	any class
`scala.collection.**`	classes in package scala.collection, or any sub-package
`com.corp.*.util.`	classes in a util package below com.corp, including com.corp.util.Foo
`**.Util`	classes named Util in any package, including `_root_.Util`
`*.Util*`	classes containing Util

There's a special case to make .**. match ., so a.**.b.C matches a.b.C. Similarly, **.A matches A.

Note that we're talking about classfile names here, so a nested class foo.C#D would be foo.C$D, a module class is scala.Predef$.

More examples:

scalac -opt:l:inline -opt-inline-from:scala.**:com.corp.**: white-listing is recommended, because
Trying to exclude everything from rt.jar is not easy (and also platform-dependent)! The following is just a start, there are many more packages: **:!java.**:!javax.**:!jdk.**:!apple.**:!com.apple.**:!com.oracle.**:!com.sun.**:...
**:!foo.**:foo.util.**: the last matching rule counts.

Fixes scala/scala-dev#148

lrytz · 2017-06-27T16:12:00Z

Feedback welcome!

Todo

jardiff, make sure the new inliner options work as intended for our own build
compilation tests with various -opt-inline-from patterns

retronym · 2017-06-28T06:23:21Z

Minor nice to have: use this new glob matcher for -Yopt-log-inline / -Yopt-trace.

retronym · 2017-06-28T06:23:56Z

Typo in commit message of c616fcb

retronym · 2017-06-28T06:27:25Z

The proposed matcher makes it possible to inadvertently inline from in a.b.c when asking for b.c. I think we should make users explicitly ask for wildcards in prefix position, so that only **.b.c would pull in both of those.

lrytz · 2017-06-28T07:15:52Z

inadvertently inline from in a.b.c when asking for b.c

I don't see what you mean here, if you say b.c, it only inlines from class c in package b, nothing else. the current implementation is what you suggest, you need **.b.c to match both b.c and a.b.c.

retronym · 2017-06-28T08:56:55Z

I must have misunderstood: "slices of qualified class names: List, scala.collection.List, collection.mutable, etc"

lrytz · 2017-06-28T08:57:43Z

Oh, that's just defining what's valid syntax

lrytz · 2017-06-28T09:02:17Z

One thing I don't like is that the description of the new setting is the longest, so it will wrap if the terminal windows is not very wide..

➜  sandbox git:(inline-from) qsc -help
Usage: scalac <options> <source files>
where possible standard options include:
  -Dproperty=value                     Pass -Dproperty=value directly to the runtime system.
...
  -opt-inline-from:<patterns>          Classfile name patterns from which to allow inlining. ** = anything, * = package or class name, ! to exclude. Example: scala.**:!scala.Predef$:corp.*.util.*:corp.**.*Util*
...
  -target:<target>                     Target platform for object files. All JVM 1.5 - 1.7 targets are deprecated. Choices: (jvm-1.5,jvm-1.6,jvm-1.7,jvm-1.8), default: jvm-1.8.

retronym · 2017-06-28T09:07:23Z

We could hide the full description of the pattern behind -opt-inline-from:help, in the same vein as:

% scala -X 2>&1 | grep lint
  -Xlint:<_,warning,-warning>      Enable or disable specific warnings: `_' for all, `-Xlint:help' to list choices.

% scala -Xlint:help
Enable or disable specific warnings
  adapted-args               Warn if an argument list is modified to match the receiver.
  nullary-unit               Warn when nullary methods return Unit.
  inaccessible               Warn about inaccessible types in method signatures.
  nullary-override           Warn when non-nullary `def f()' overrides nullary `def f'.

lrytz · 2017-06-28T09:08:41Z

yeah, that will require some work on the settings infra, StringSetting doesn't support help currently

retronym · 2017-06-28T09:28:35Z

src/compiler/scala/tools/nsc/settings/ScalaSettings.scala

+    "-opt-inline-from",
+    "patterns",
+    "Classfile name patterns from which to allow inlining. ** = anything, * = package or class name, ! to exclude. Example: scala.**:!scala.Predef$:corp.*.util.*:corp.**.*Util*",
+    "")


Perhaps this could be a MultiStringSetting to allow multiple -opt-inline-from entries on the command line to aggregate.

scaladoc -implicits-hide is an example of a similar part of the settings UI that goes that way.

I'm happy to do it either way, but I don't see a big advantage of switching to MultiStringSetting

I think the use case is to make it easy to use some base set of compiler options (e.g shared between all subprojects), and then append an extra option for some specific subproject:

scalacOptions in sub1 += List("-opt-inline-from", "...")

Without MultiStringSetting, I beleive you'd need to perform a more complicated merge:

scalacOptions = { val i = scalacOptions.value.indexOf("-opt-inline-from" if (i < 0) scalacOptions.value ++ List("-opt-inline-from", "...") else scalacOptions.value.patch(i, Nil, 2) ++ List("-opt-inline-from", scalacOptions.value(i + 1) + ",...")

OK, that's a good point, I'll change it. The potentially confusing thing about a MultiStringSetting is that if you're using it in the last position and not in "colonated" mode, it consumes the source files. For example:

➜ sandbox git:(inline-from) ✗ scaladoc -implicits-hide Foo Bar Test.scala >> shows usage ➜ sandbox git:(inline-from) ✗ scaladoc -implicits-hide Foo Bar -d . Test.scala >> runs scaladoc

Hmm, that's annoying. If you know about it, you can use -- to end the opton section and move into source files.

% scaladoc -implicits-hide foo bar -- /tmp/test.scala

And the help text also guides you towards the :.

-implicits-hide:<implicit(s)> Hide the members inherited by the given comma separated, fully qualified implicit conversions. Add dot (.) to include default conversions.

Ah, I tried a single - as separator, but that didn't work. I'll make sure the help text recommends the colonated mode.

Introduce the optimizer level `-opt:l:inline` and a new setting `-opt-inline-from` to control what classes we inline from. `-opt:l:classpath` and `-opt:l:project` continue to work in the same way, with a deprecation warning.

Allows StringSetting to be helping.

lrytz · 2017-06-29T06:28:50Z

➜  sandbox git:(inline-from) qsc -help
...
  -opt-inline-from <patterns>          Patterns for classfile names from which to allow inlining, `help` for details.
...
➜  sandbox git:(inline-from) qsc -opt-inline-from help
Patterns for classfile names from which the inliner is allowed to pull in code.
  *              Matches classes in the empty package
  **             All classes
  a.C            Class a.C
  a.*            Classes in package a
  a.**           Classes in a and in sub-packages of a
  **.Util        Classes named Util in any package (including the empty package)
  a.**.*Util*    Classes in a and sub-packages with Util in their name (including a.Util)
  a.C$D          The nested class D defined in class a.C
  scala.Predef$  The scala.Predef object

The setting accepts a colon-separated list of patterns. A leading `!` marks a pattern excluding.
The last matching pattern defines whether a classfile is included or excluded (default: excluded).
For example, `a.**:!a.b.**` includes classes in a and sub-packages, but not in a.b and sub-packages.

Note: on the command-line you might need to quote patterns containing `*` to prevent the shell
from expanding it to a list of files in the current directory.
➜  sandbox git:(inline-from)

lrytz · 2017-06-29T11:01:41Z

Jardiff: I created a locker from 0c51534 (the current tip of this PR). Then:

using -Dstarr.version=2.12.3-bin-2c50123 (the branch point of this PR, build from scala-integration) with -opt:l:classpath, built 2.12.3-bin-0c515344a5-quick-a
using -Dstarr.version=2.12.3-bin-0c515344a5-locker with Seq("-opt:l:inline", "-opt-inline-from", "scala/**"), built 2.12.3-bin-0c515344a5-quick-b

for l in library reflect compiler; do echo ">>> $l <<<"; jardiff /Users/luc/.ivy2/local/org.scala-lang/scala-$l/2.12.3-bin-0c515344a5-quick-a/jars/scala-$l.jar /Users/luc/.ivy2/local/org.scala-lang/scala-$l/2.12.3-bin-0c515344a5-quick-b/jars/scala-$l.jar; done

Result here: a number of methods from the rt.jar that are no longer inlined, for example java/util/Iterator.forEachRemaining.

I also compiled lib+reflect+compiler with -Yopt-log-inline _ -opt:l:inline -opt-inline-from 'scala/**' find ../src/library ../src/reflect ../src/compiler -name '*.scala' -d a and checked the output to make sure only code from scala/ is inlined.

For curiosity, here's a histogram of inlined methods: https://gist.github.com/lrytz/6f689a78e497e7ebed966be0bcb0a36f

retronym · 2017-06-29T11:11:20Z

Jardiff: I created a locker from 0c51534 (the current tip of this PR). Then ...

FYI, I'm working on better automation for this sort of analysis

2.12.x...retronym:topic/jardiff

lrytz · 2017-06-29T16:07:41Z

I think this is ready now.

retronym · 2017-06-29T23:09:41Z

src/compiler/scala/tools/nsc/settings/ScalaSettings.scala

@@ -254,7 +254,7 @@ trait ScalaSettings extends AbsScalaSettings

    private val inlineChoices = List(lMethod, inline)
    val lInline = Choice("l:inline",
-      "Enable cross-method optimizations: " + inlineChoices.mkString("", ",", "."),
+      "Enable cross-method optimizations (note: inlining requires -opt-inlnie-from): " + inlineChoices.mkString("", ",", "."),


typo here.

Just checking: do we inline unconditionally from sources in the current run, even without -opt-inlnie-from?

Pros: a convenient default without needing
Cons: instable results depending on separate compilation

... I'd prefer the stability over convenience, I think.

I agree, and that's the current implementation. We could bring back the functionality of -opt:l:project with a special pattern in -opt-inline-from:...:<sources>:...

Allows passing the setting multiple times, which might be useful for multi-project builds

Move jvm-specific settings from scalaModuleSettings to a new scalaModuleSettingsJVM setting. This means that JVM projects in scala modules now have to add bot `scalaModuleSettings` and `scalaModuleSettingsJVM`. Adding sbt-osgi settings in a scala-js project caused the published jar to be empty (scala/scala-parser-combinators#119). Upgrades sbt-osgi to the latest version. Also anticipates the new optimizer settings in 2.12.3 (scala/scala#5964).

See scala/scala#5964

er1c · 2017-10-16T22:11:12Z

The scalac doc shows : as the delimiter, but I believe this was actually implemented with a , comma - is this difference being tracked somewhere?

lrytz · 2017-10-17T05:15:17Z

Patterns are separated by :. There was a mistake in the PR description here, I fixed that.

scala-jenkins added this to the 2.12.3 milestone Jun 27, 2017

SethTisue added the release-notes worth highlighting in next release notes label Jun 27, 2017

retronym reviewed Jun 28, 2017

View reviewed changes

lrytz added 4 commits June 28, 2017 23:03

parser and checker for -opt-inline-from setting

1cebf8f

Extensive help text for -opt-inline-from

be5126f

Allows StringSetting to be helping.

Tests for compilation with various -opt-inline-from settings

0c51534

lrytz force-pushed the inline-from branch from f340924 to 0c51534 Compare June 28, 2017 21:06

retronym reviewed Jun 29, 2017

View reviewed changes

Use MultiStringSetting for -opt-inline-from

dddf5a2

Allows passing the setting multiple times, which might be useful for multi-project builds

lrytz force-pushed the inline-from branch from 3457ddb to dddf5a2 Compare June 30, 2017 07:17

retronym approved these changes Jul 3, 2017

View reviewed changes

retronym merged commit 343a199 into scala:2.12.x Jul 3, 2017

lrytz mentioned this pull request Jul 6, 2017

Split jvm-specific settings into scalaModuleSettingsJVM scala/sbt-scala-module#27

Merged

retronym mentioned this pull request Jul 7, 2017

Release 2.12.3 scala/scala-dev#404

Closed

37 tasks

lrytz deleted the inline-from branch July 20, 2017 14:27

jbgi mentioned this pull request Jul 28, 2017

switch to scala 2.12 and add scaffolding for scala.js scalaz/scalaz#1426

Merged

6 tasks

dwijnand mentioned this pull request Aug 1, 2017

NoSuchMethodError when the metabuild scalaVersion is 2.12.3 lightbend/mima#191

Closed

lrytz mentioned this pull request Aug 12, 2017

Remove workarounds for sbt/sbt#1035 sbt/zinc#393

Merged

eparejatobes added a commit to ohnosequences/stuff that referenced this pull request Aug 12, 2017

Update build def to new inline settings

4588a5a

See scala/scala#5964

lrytz mentioned this pull request Sep 5, 2017

Using the inliner (-opt:l:classpath) causes binary incompatible releases twitter/algebird#640

Closed

lrytz mentioned this pull request Mar 7, 2018

Provide more control, better defaults, for cross-library inlining scala/bug#7793

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New setting `-opt-inline-from` to control where to inline from #5964

New setting `-opt-inline-from` to control where to inline from #5964

lrytz commented Jun 27, 2017 •

edited

lrytz commented Jun 27, 2017 •

edited

retronym commented Jun 28, 2017

retronym commented Jun 28, 2017

retronym commented Jun 28, 2017

lrytz commented Jun 28, 2017

retronym commented Jun 28, 2017

lrytz commented Jun 28, 2017

lrytz commented Jun 28, 2017 •

edited

retronym commented Jun 28, 2017

lrytz commented Jun 28, 2017

retronym Jun 28, 2017

lrytz Jun 28, 2017

retronym Jun 29, 2017 •

edited

lrytz Jun 29, 2017 •

edited

retronym Jun 29, 2017

lrytz Jun 29, 2017

lrytz commented Jun 29, 2017

lrytz commented Jun 29, 2017

retronym commented Jun 29, 2017

lrytz commented Jun 29, 2017

retronym Jun 29, 2017

retronym Jun 29, 2017

lrytz Jun 30, 2017

er1c commented Oct 16, 2017

lrytz commented Oct 17, 2017

New setting -opt-inline-from to control where to inline from #5964

New setting -opt-inline-from to control where to inline from #5964

Conversation

lrytz commented Jun 27, 2017 • edited

lrytz commented Jun 27, 2017 • edited

retronym commented Jun 28, 2017

retronym commented Jun 28, 2017

retronym commented Jun 28, 2017

lrytz commented Jun 28, 2017

retronym commented Jun 28, 2017

lrytz commented Jun 28, 2017

lrytz commented Jun 28, 2017 • edited

retronym commented Jun 28, 2017

lrytz commented Jun 28, 2017

retronym Jun 28, 2017

Choose a reason for hiding this comment

lrytz Jun 28, 2017

Choose a reason for hiding this comment

retronym Jun 29, 2017 • edited

Choose a reason for hiding this comment

lrytz Jun 29, 2017 • edited

Choose a reason for hiding this comment

retronym Jun 29, 2017

Choose a reason for hiding this comment

lrytz Jun 29, 2017

Choose a reason for hiding this comment

lrytz commented Jun 29, 2017

lrytz commented Jun 29, 2017

retronym commented Jun 29, 2017

lrytz commented Jun 29, 2017

retronym Jun 29, 2017

Choose a reason for hiding this comment

retronym Jun 29, 2017

Choose a reason for hiding this comment

lrytz Jun 30, 2017

Choose a reason for hiding this comment

er1c commented Oct 16, 2017

lrytz commented Oct 17, 2017

New setting `-opt-inline-from` to control where to inline from #5964

New setting `-opt-inline-from` to control where to inline from #5964

lrytz commented Jun 27, 2017 •

edited

lrytz commented Jun 27, 2017 •

edited

lrytz commented Jun 28, 2017 •

edited

retronym Jun 29, 2017 •

edited

lrytz Jun 29, 2017 •

edited