Add optional ability to use Turbine #165

treblereel · 2022-05-03T22:30:45Z

Turbine is a header compiler for Java that allows us to reduce the
source code to parse, leading to decreased compile time. By ignoring
changes in the body, we can also avoid downstream builds that can be
skipped.

This feature is optional for now, but may be set to the default in a
future release. To enable this, assign the taskMappings key of
stripped_bytecode_headers to the value turbine.

Co-authored-by: Colin Alworth colin@vertispan.com
Fixes #150

treblereel · 2022-05-05T01:28:35Z

i did few perf test on my mac mini m1 16gb:

i used my crysknife demo that contains elemental2-* and many gwtproject modules including heavy like gwt-widgets

with turbine:

(base) treblereel@Dmitiis-Mac-mini demo % hyperfine --runs 10 'mvn clean package'
Benchmark 1: mvn clean package
Time (mean ± σ): 97.681 s ± 1.508 s [User: 356.236 s, System: 268.646 s]
Range (min … max): 95.731 s … 100.939 s 10 runs

without :

Benchmark 1: mvn clean package
Time (mean ± σ): 103.222 s ± 0.706 s [User: 371.494 s, System: 274.101 s]
Range (min … max): 102.359 s … 104.468 s 10 runs

as @niloc132 can see, tubine is slightly faster, the main bottle neck is ADVANCED comp mode.

So i think, adding turbine makes sense only if we want to have incremental mode. The good news, turbine isn't slower.

niloc132 · 2022-05-05T13:26:31Z

Excellent - I wasn't hoping for performance improvements here, but that this would instead make non-clean builds faster with only some modules changing.

treblereel · 2022-05-06T20:45:25Z

@niloc132 few notes:

https://github.com/google/turbine/blob/dd578b81d048aff7bab4d4772eb37ef11d5cd3b2/java/com/google/turbine/options/TurbineOptions.java#L153 expects us to provide set of jars, so for compileClasspath i provide *-stripped_bytecode_headers/results/output.jar
Maybe we should tweak turbine to allow it to process raw *.class files
https://github.com/google/turbine/blob/dd578b81d048aff7bab4d4772eb37ef11d5cd3b2/java/com/google/turbine/options/TurbineOptions.java#L34 should be investigated
and, i know how you like it, we have issues with apt again. If apt is presented in reactor, turbine can't process if because it can't find all deps ... I skip this exception for now.

niloc132 · 2022-05-06T20:58:09Z

As long as you are only focusing on the stripped-bytecode and stripped-bytecode-headers, we don't care about APT, that is already finished. It would make sense later to follow up and work on the APT also, but that's an earlier phase of the build, and we're still using javac for that.

If this task fails, whether turbine or javac, we can safely ignore it, since APT isn't necessary (and we can't tell which dependencies are apt-only, which are actual runtime deps).

--

if jars are not optional, we can either tweak how j2cl runs to use that jar (assuming that the jar is 100% deterministic...), or we can add a "make a jar before compiling" like how you unzip afterwards. Not pretty, but it will work, and probably still worth it performance-wise.

Definitely should consider patching turbine to support just reading off the filesystem though.

Bumps [maven-shared-utils](https://github.com/apache/maven-shared-utils) from 3.1.0 to 3.3.3. - [Release notes](https://github.com/apache/maven-shared-utils/releases) - [Commits](apache/maven-shared-utils@maven-shared-utils-3.1.0...maven-shared-utils-3.3.3) --- updated-dependencies: - dependency-name: org.apache.maven.shared:maven-shared-utils dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

niloc132 · 2022-06-12T18:09:59Z

Update from discussion in gitter: we're going to have this feature optional (but disabled by default in 0.20), and require java11 rather than add the java version check.

Starting in 0.21, the tooling will require java11, since closure compiler is also dropping java8 support. Turbine will likely be enabled by default starting in 0.21.

treblereel · 2022-06-12T21:10:49Z

j2cl-tasks/src/main/java/com/vertispan/j2cl/build/provided/TurbineTask.java

+
+    @Override
+    public Task resolve(Project project, Config config) {
+        int version = getJavaVersion();


@niloc132 so we keep it for now and ll remove it in 0.21, right ?

Sure - we could probably just fail now too though, your call.

let simplify it for now

niloc132 · 2022-06-20T18:07:52Z

j2cl-tasks/src/main/java/com/vertispan/j2cl/build/provided/TurbineTask.java

+                                .setSources(ImmutableList.copyOf(sources))
+                                .setOutput(output.toString())
+                                .setClassPath(ImmutableList.copyOf(deps))
+                                //TODO take a look at ReducedClasspathMode


ticket for TODO

ticket is created

niloc132 · 2022-06-20T18:08:10Z

j2cl-tasks/src/main/java/com/vertispan/j2cl/build/provided/TurbineTask.java

+                extractJar(output.toString(), resultFolder.toString());
+            } catch (TurbineError e) {
+                //ohhhh apt is in maven reactor
+                System.out.println(e.getMessage());


use the logger in the context, or just drop the error?

niloc132 · 2022-06-20T18:47:04Z

build-caching/src/main/java/com/vertispan/j2cl/build/Dependency.java

@@ -44,4 +48,12 @@ public Scope getScope() {
    public void setScope(Scope scope) {
        this.scope = scope;
    }
+
+    public File getJar() {


This seems to be unused now, read but never written - can we revert this?

j2cl-tasks/src/main/java/com/vertispan/j2cl/build/provided/TurbineTask.java

niloc132 · 2022-06-20T19:02:43Z

j2cl-tasks/src/main/java/com/vertispan/j2cl/build/provided/TurbineTask.java

+        };
+    }
+
+    public void extractJar(String zipFilePath, String extractDirectory) {


Take a quick look at 22319e6? I used that zip unpacking code originally to deal with j2cl tools that didnt know how to use the filesystem directly. That would let us drop the new commons-compress dependency.

build-caching/src/main/java/com/vertispan/j2cl/build/Dependency.java

j2cl-tasks/src/main/java/com/vertispan/j2cl/build/provided/TurbineTask.java

niloc132 · 2022-06-23T17:10:38Z

pom.xml

@@ -105,6 +105,7 @@
    <error.prone.annotations.version>2.1.3</error.prone.annotations.version>
    <jsr305.version>3.0.2</jsr305.version>
    <commons.lang3.version>3.8.1</commons.lang3.version>
+    <google.turbine.version>v20220503-SNAPSHOT</google.turbine.version>


I'll finish this release so you can skip SNAPSHOT here, since it looks like we're solid, just some future plans to stop requiring a zip right?

…y.java Co-authored-by: Colin Alworth <colin@vertispan.com>

…bineTask.java Co-authored-by: Colin Alworth <colin@vertispan.com>

treblereel requested a review from niloc132 May 4, 2022 02:46

treblereel self-assigned this May 4, 2022

treblereel added the enhancement New feature or request label May 4, 2022

treblereel added this to the 0.20 milestone May 4, 2022

niloc132 mentioned this pull request May 5, 2022

java bytecode ijar/turbine research work #150

Closed

niloc132 and others added 4 commits May 6, 2022 13:18

Quick stub to try adding turbine, force elemental2-project to use it

0b02a98

initial google turbine implementation

08e2fc3

use turbine from https://repo.vertispan.com/j2cl/

e4fa35f

few updates

a1a5926

treblereel force-pushed the turbine-stub branch from 0a5f129 to a1a5926 Compare May 6, 2022 20:36

dependabot bot and others added 3 commits May 28, 2022 19:29

poms improved

bdf9362

fallback TurbineTask to JavacTask if java version is 1.8

0025d80

treblereel commented Jun 12, 2022

View reviewed changes

niloc132 reviewed Jun 20, 2022

View reviewed changes

after review

283d18b

niloc132 reviewed Jun 23, 2022

View reviewed changes

treblereel and others added 5 commits June 23, 2022 10:28

Update build-caching/src/main/java/com/vertispan/j2cl/build/Dependenc…

f5b994f

…y.java Co-authored-by: Colin Alworth <colin@vertispan.com>

Update j2cl-tasks/src/main/java/com/vertispan/j2cl/build/provided/Tur…

087b2a7

…bineTask.java Co-authored-by: Colin Alworth <colin@vertispan.com>

Update j2cl-tasks/src/main/java/com/vertispan/j2cl/build/provided/Tur…

6fb83bb

…bineTask.java Co-authored-by: Colin Alworth <colin@vertispan.com>

Update j2cl-tasks/src/main/java/com/vertispan/j2cl/build/provided/Tur…

30b6eb2

…bineTask.java Co-authored-by: Colin Alworth <colin@vertispan.com>

fixed typo

4279d6f

treblereel changed the title ~~Turbine stub~~ Add optional ability to use Turbine Jun 23, 2022

switch turbine to publicaly released

f8c42c7

niloc132 approved these changes Jun 24, 2022

View reviewed changes

niloc132 merged commit ff6b1ae into Vertispan:main Jun 24, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add optional ability to use Turbine #165

Add optional ability to use Turbine #165

treblereel commented May 3, 2022 •

edited by niloc132

Loading

treblereel commented May 5, 2022

niloc132 commented May 5, 2022

treblereel commented May 6, 2022

niloc132 commented May 6, 2022

niloc132 commented Jun 12, 2022

treblereel Jun 12, 2022

niloc132 Jun 20, 2022

treblereel Jun 23, 2022

niloc132 Jun 20, 2022

treblereel Jun 23, 2022

niloc132 Jun 20, 2022

treblereel Jun 23, 2022

niloc132 Jun 20, 2022

niloc132 Jun 20, 2022

niloc132 Jun 23, 2022

Add optional ability to use Turbine #165

Add optional ability to use Turbine #165

Conversation

treblereel commented May 3, 2022 • edited by niloc132 Loading

treblereel commented May 5, 2022

niloc132 commented May 5, 2022

treblereel commented May 6, 2022

niloc132 commented May 6, 2022

niloc132 commented Jun 12, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

treblereel commented May 3, 2022 •

edited by niloc132

Loading