From e82e47e68e8d65823339db131a38ec4cb5c84983 Mon Sep 17 00:00:00 2001 From: James Duong Date: Fri, 24 Nov 2023 15:22:34 -0800 Subject: [PATCH 01/30] Add and integrate the module-info compiler plugin Note that some configuration from the arrow root POM is copied to the maven plugins module because it cannot be inherited without creating a cyclic depdendency. --- .../module-info-compiler-maven-plugin/pom.xml | 131 ++++++++ .../plugins/ModuleInfoCompilerPlugin.java | 93 ++++++ java/maven/pom.xml | 308 ++++++++++++++++++ java/pom.xml | 18 +- 4 files changed, 549 insertions(+), 1 deletion(-) create mode 100644 java/maven/module-info-compiler-maven-plugin/pom.xml create mode 100644 java/maven/module-info-compiler-maven-plugin/src/main/java/org/apache/arrow/maven/plugins/ModuleInfoCompilerPlugin.java create mode 100644 java/maven/pom.xml diff --git a/java/maven/module-info-compiler-maven-plugin/pom.xml b/java/maven/module-info-compiler-maven-plugin/pom.xml new file mode 100644 index 00000000000..be2a9f812d3 --- /dev/null +++ b/java/maven/module-info-compiler-maven-plugin/pom.xml @@ -0,0 +1,131 @@ + + + + 4.0.0 + + org.apache.arrow.maven.plugins + arrow-maven-plugins + 15.0.0-SNAPSHOT + + module-info-compiler-maven-plugin + 15.0.0-SNAPSHOT + maven-plugin + + Module Info Compiler Maven Plugin + + https://arrow.apache.org + + + ${maven.version} + + + + 3.3.9 + + + + + org.glavo + module-info-compiler + 2.0 + + + org.apache.maven + maven-plugin-api + ${maven.version} + provided + + + org.apache.maven + maven-core + ${maven.version} + provided + + + org.apache.maven + maven-artifact + ${maven.version} + provided + + + org.apache.maven + maven-model + ${maven.version} + provided + + + org.apache.maven.plugin-tools + maven-plugin-annotations + 3.6.0 + provided + + + + + + + + maven-clean-plugin + 3.1.0 + + + maven-plugin-plugin + 3.6.0 + + + maven-jar-plugin + 3.0.2 + + + maven-install-plugin + 2.5.2 + + + maven-deploy-plugin + 2.8.2 + + + maven-invoker-plugin + 3.1.0 + + + + + + org.apache.maven.plugins + maven-plugin-plugin + 3.6.0 + + true + + + + mojo-descriptor + + descriptor + + + + help-goal + + helpmojo + + + + + + + + + \ No newline at end of file diff --git a/java/maven/module-info-compiler-maven-plugin/src/main/java/org/apache/arrow/maven/plugins/ModuleInfoCompilerPlugin.java b/java/maven/module-info-compiler-maven-plugin/src/main/java/org/apache/arrow/maven/plugins/ModuleInfoCompilerPlugin.java new file mode 100644 index 00000000000..8f1baf8f94b --- /dev/null +++ b/java/maven/module-info-compiler-maven-plugin/src/main/java/org/apache/arrow/maven/plugins/ModuleInfoCompilerPlugin.java @@ -0,0 +1,93 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.arrow.maven.plugins; + +import java.io.File; +import java.io.IOException; +import java.io.InputStreamReader; +import java.io.OutputStream; +import java.io.Reader; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; +import java.util.Optional; + +import org.apache.maven.plugin.AbstractMojo; +import org.apache.maven.plugin.MojoExecutionException; +import org.apache.maven.plugins.annotations.LifecyclePhase; +import org.apache.maven.plugins.annotations.Mojo; +import org.apache.maven.plugins.annotations.Parameter; +import org.apache.maven.project.MavenProject; +import org.glavo.mic.ModuleInfoCompiler; + +/** + * Compiles the first module-info.java file in the project purely syntactically. + */ +@Mojo(name = "module-info-compile", defaultPhase = LifecyclePhase.COMPILE) +public class ModuleInfoCompilerPlugin extends AbstractMojo { + /** + * Source directories. + */ + @Parameter(defaultValue = "${project.compileSourceRoots}", property = "compileSourceRoots", + required = true) + private final List compileSourceRoots = new ArrayList<>(); + + @Parameter(defaultValue = "${project}", readonly = true, required = true) + private MavenProject project; + + @Override + public void execute() throws MojoExecutionException { + Optional moduleInfoFile = findFirstModuleInfo(compileSourceRoots); + if (moduleInfoFile.isPresent()) { + // The compiled module-info.class file goes into target/classes/module-info/main + Path outputDir = Path.of(project.getBuild().getOutputDirectory()); + + outputDir.toFile().mkdirs(); + Path targetPath = outputDir.resolve("module-info.class"); + + // Invoke the compiler, + ModuleInfoCompiler compiler = new ModuleInfoCompiler(); + try (Reader reader = new InputStreamReader(Files.newInputStream(moduleInfoFile.get().toPath()), + StandardCharsets.UTF_8); + OutputStream output = Files.newOutputStream(targetPath)) { + compiler.compile(reader, output); + } catch (IOException ex) { + throw new MojoExecutionException("Error compiling module-info.java", ex); + } + } + } + + /** + * Finds the first module-info.java file in the set of source directories. + */ + private Optional findFirstModuleInfo(List sourceDirectories) { + if (sourceDirectories == null) { + return Optional.empty(); + } + + return sourceDirectories.stream().map(Path::of) + .map(sourcePath -> + sourcePath.toFile().listFiles(file -> + file.getName().equals("module-info.java"))) + .filter(matchingFiles -> matchingFiles != null && matchingFiles.length != 0) + .map(matchingFiles -> matchingFiles[0]) + .findAny(); + } +} diff --git a/java/maven/pom.xml b/java/maven/pom.xml new file mode 100644 index 00000000000..f8c574d203c --- /dev/null +++ b/java/maven/pom.xml @@ -0,0 +1,308 @@ + + + + 4.0.0 + + org.apache.arrow.maven.plugins + arrow-maven-plugins + 15.0.0-SNAPSHOT + Arrow Maven Plugins + pom + + + module-info-compiler-maven-plugin + + + + true + + + + + + org.apache.rat + apache-rat-plugin + + + rat-checks + validate + + check + + + + + false + + **/dependency-reduced-pom.xml + **/*.log + **/*.css + **/*.js + **/*.md + **/*.eps + **/*.json + **/*.seq + **/*.parquet + **/*.sql + **/arrow-git.properties + **/*.csv + **/*.csvh + **/*.csvh-test + **/*.tsv + **/*.txt + **/*.ssv + **/arrow-*.conf + **/.buildpath + **/*.proto + **/*.fmpp + **/target/** + **/*.tdd + **/*.project + **/TAGS + **/*.checkstyle + **/.classpath + **/.factorypath + **/.settings/** + .*/** + **/*.patch + **/*.pb.cc + **/*.pb.h + **/*.linux + **/client/build/** + **/*.tbl + **/*.iml + **/flight.properties + **/*.idea/** + + + + + + org.apache.maven.plugins + maven-jar-plugin + + + **/logging.properties + **/logback-test.xml + **/logback.out.xml + **/logback.xml + + + true + + true + true + + + org.apache.arrow + ${username} + https://arrow.apache.org/ + + + + + + + test-jar + + + true + + + + + + + + org.apache.maven.plugins + maven-resources-plugin + + UTF-8 + + + + org.apache.maven.plugins + maven-compiler-plugin + + 1.8 + 1.8 + 2048m + false + true + + + + maven-enforcer-plugin + + + validate_java_and_maven_version + verify + + enforce + + false + + + + [3.3.0,4) + + + + + + avoid_bad_dependencies + verify + + enforce + + + + + + commons-logging + javax.servlet:servlet-api + org.mortbay.jetty:servlet-api + org.mortbay.jetty:servlet-api-2.5 + log4j:log4j + + + + + + + + + pl.project13.maven + git-commit-id-plugin + 4.0.5 + + + for-jars + true + + revision + + + target/classes/arrow-git.properties + + + + for-source-tarball + + revision + + false + + ./arrow-git.properties + + + + + + dd.MM.yyyy '@' HH:mm:ss z + false + false + true + false + + false + false + 7 + -dirty + true + + + + + + org.apache.maven.plugins + maven-checkstyle-plugin + 3.1.0 + + + com.puppycrawl.tools + checkstyle + 8.19 + + + org.slf4j + jcl-over-slf4j + 1.7.5 + + + + + validate + validate + + check + + + + + ../dev/checkstyle/checkstyle.xml + ../dev/checkstyle/checkstyle.license + ../dev/checkstyle/suppressions.xml + true + UTF-8 + true + ${checkstyle.failOnViolation} + ${checkstyle.failOnViolation} + warning + xml + html + ${project.build.directory}/test/checkstyle-errors.xml + false + + + + org.apache.maven.plugins + maven-dependency-plugin + + + analyze + verify + + analyze-only + + + true + true + + + javax.annotation:javax.annotation-api:* + org.apache.hadoop:hadoop-client-api + + + + + + + org.cyclonedx + cyclonedx-maven-plugin + 2.7.10 + + + package + + makeBom + + + + + + + + diff --git a/java/pom.xml b/java/pom.xml index a9b80ccdb5d..bd0f1e46445 100644 --- a/java/pom.xml +++ b/java/pom.xml @@ -371,6 +371,17 @@ + + org.apache.arrow.maven.plugins + module-info-compiler-maven-plugin + + + + module-info-compile + + + + @@ -546,6 +557,11 @@ + + org.apache.arrow.maven.plugins + module-info-compiler-maven-plugin + ${project.version} + @@ -735,6 +751,7 @@ + maven bom format memory @@ -1236,7 +1253,6 @@ - From c1c633031cda4c025b2f20b1cc0f5ded7dc69e18 Mon Sep 17 00:00:00 2001 From: James Duong Date: Fri, 24 Nov 2023 15:49:10 -0800 Subject: [PATCH 02/30] Fix JDK8 compile issue with module-info-compiler and attempt to fix analyze issue --- java/maven/module-info-compiler-maven-plugin/pom.xml | 1 + .../apache/arrow/maven/plugins/ModuleInfoCompilerPlugin.java | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/java/maven/module-info-compiler-maven-plugin/pom.xml b/java/maven/module-info-compiler-maven-plugin/pom.xml index be2a9f812d3..ac2b41621db 100644 --- a/java/maven/module-info-compiler-maven-plugin/pom.xml +++ b/java/maven/module-info-compiler-maven-plugin/pom.xml @@ -39,6 +39,7 @@ org.glavo module-info-compiler 2.0 + jar org.apache.maven diff --git a/java/maven/module-info-compiler-maven-plugin/src/main/java/org/apache/arrow/maven/plugins/ModuleInfoCompilerPlugin.java b/java/maven/module-info-compiler-maven-plugin/src/main/java/org/apache/arrow/maven/plugins/ModuleInfoCompilerPlugin.java index 8f1baf8f94b..93e99e9f164 100644 --- a/java/maven/module-info-compiler-maven-plugin/src/main/java/org/apache/arrow/maven/plugins/ModuleInfoCompilerPlugin.java +++ b/java/maven/module-info-compiler-maven-plugin/src/main/java/org/apache/arrow/maven/plugins/ModuleInfoCompilerPlugin.java @@ -25,6 +25,7 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.Paths; import java.util.ArrayList; import java.util.List; import java.util.Optional; @@ -57,7 +58,7 @@ public void execute() throws MojoExecutionException { Optional moduleInfoFile = findFirstModuleInfo(compileSourceRoots); if (moduleInfoFile.isPresent()) { // The compiled module-info.class file goes into target/classes/module-info/main - Path outputDir = Path.of(project.getBuild().getOutputDirectory()); + Path outputDir = Paths.get(project.getBuild().getOutputDirectory()); outputDir.toFile().mkdirs(); Path targetPath = outputDir.resolve("module-info.class"); From c5eaceb2fa6e266e773470cfede6ade9245a63a0 Mon Sep 17 00:00:00 2001 From: James Duong Date: Fri, 24 Nov 2023 15:52:25 -0800 Subject: [PATCH 03/30] Fix JDK8 issue with use of Path.of --- .../apache/arrow/maven/plugins/ModuleInfoCompilerPlugin.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/maven/module-info-compiler-maven-plugin/src/main/java/org/apache/arrow/maven/plugins/ModuleInfoCompilerPlugin.java b/java/maven/module-info-compiler-maven-plugin/src/main/java/org/apache/arrow/maven/plugins/ModuleInfoCompilerPlugin.java index 93e99e9f164..e0475c27ee0 100644 --- a/java/maven/module-info-compiler-maven-plugin/src/main/java/org/apache/arrow/maven/plugins/ModuleInfoCompilerPlugin.java +++ b/java/maven/module-info-compiler-maven-plugin/src/main/java/org/apache/arrow/maven/plugins/ModuleInfoCompilerPlugin.java @@ -83,7 +83,7 @@ private Optional findFirstModuleInfo(List sourceDirectories) { return Optional.empty(); } - return sourceDirectories.stream().map(Path::of) + return sourceDirectories.stream().map(Paths::get) .map(sourcePath -> sourcePath.toFile().listFiles(file -> file.getName().equals("module-info.java"))) From 1b73e2431ff6cece7020e17feddf0fe35fd2057f Mon Sep 17 00:00:00 2001 From: James Duong Date: Fri, 24 Nov 2023 16:02:30 -0800 Subject: [PATCH 04/30] Skip the maven-dependency-plugin analyze check due to false positive Reports a used dependency as unused only on Windows 2022 CI runs --- .../module-info-compiler-maven-plugin/pom.xml | 23 ++++++++++++++++++- java/maven/pom.xml | 1 + 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/java/maven/module-info-compiler-maven-plugin/pom.xml b/java/maven/module-info-compiler-maven-plugin/pom.xml index ac2b41621db..b15d89eb5b3 100644 --- a/java/maven/module-info-compiler-maven-plugin/pom.xml +++ b/java/maven/module-info-compiler-maven-plugin/pom.xml @@ -39,7 +39,6 @@ org.glavo module-info-compiler 2.0 - jar org.apache.maven @@ -103,6 +102,28 @@ + + org.apache.maven.plugins + maven-dependency-plugin + + + analyze + verify + + analyze-only + + + + true + + + + org.apache.maven.plugins maven-plugin-plugin diff --git a/java/maven/pom.xml b/java/maven/pom.xml index f8c574d203c..3119fc34187 100644 --- a/java/maven/pom.xml +++ b/java/maven/pom.xml @@ -136,6 +136,7 @@ org.apache.maven.plugins maven-compiler-plugin + UTF-8 1.8 1.8 2048m From 9894c9294f3f99b782a70e10b2a93a642642d24c Mon Sep 17 00:00:00 2001 From: James Duong Date: Sun, 26 Nov 2023 06:57:35 -0800 Subject: [PATCH 05/30] Fix non-inherited version --- java/maven/module-info-compiler-maven-plugin/pom.xml | 1 - 1 file changed, 1 deletion(-) diff --git a/java/maven/module-info-compiler-maven-plugin/pom.xml b/java/maven/module-info-compiler-maven-plugin/pom.xml index b15d89eb5b3..6cd72edb095 100644 --- a/java/maven/module-info-compiler-maven-plugin/pom.xml +++ b/java/maven/module-info-compiler-maven-plugin/pom.xml @@ -19,7 +19,6 @@ 15.0.0-SNAPSHOT module-info-compiler-maven-plugin - 15.0.0-SNAPSHOT maven-plugin Module Info Compiler Maven Plugin From 348011b80ee064855242b3378035040415ddac75 Mon Sep 17 00:00:00 2001 From: James Duong Date: Sun, 26 Nov 2023 07:00:00 -0800 Subject: [PATCH 06/30] Do not use dependency analyze when compiling plugins It is not correctly getting skipped. --- .../module-info-compiler-maven-plugin/pom.xml | 22 ------------------- java/maven/pom.xml | 22 ------------------- 2 files changed, 44 deletions(-) diff --git a/java/maven/module-info-compiler-maven-plugin/pom.xml b/java/maven/module-info-compiler-maven-plugin/pom.xml index 6cd72edb095..ce598fc7f0f 100644 --- a/java/maven/module-info-compiler-maven-plugin/pom.xml +++ b/java/maven/module-info-compiler-maven-plugin/pom.xml @@ -101,28 +101,6 @@ - - org.apache.maven.plugins - maven-dependency-plugin - - - analyze - verify - - analyze-only - - - - true - - - - org.apache.maven.plugins maven-plugin-plugin diff --git a/java/maven/pom.xml b/java/maven/pom.xml index 3119fc34187..86ac402732b 100644 --- a/java/maven/pom.xml +++ b/java/maven/pom.xml @@ -268,28 +268,6 @@ false - - org.apache.maven.plugins - maven-dependency-plugin - - - analyze - verify - - analyze-only - - - true - true - - - javax.annotation:javax.annotation-api:* - org.apache.hadoop:hadoop-client-api - - - - - org.cyclonedx cyclonedx-maven-plugin From 50fe6d72ed25c06193b3c816b7e7f239ee5b5f5a Mon Sep 17 00:00:00 2001 From: James Duong Date: Wed, 29 Nov 2023 15:59:04 -0800 Subject: [PATCH 07/30] Exclude module-info.java from compilation This won't build with the JDK 8 target --- java/pom.xml | 1 + 1 file changed, 1 insertion(+) diff --git a/java/pom.xml b/java/pom.xml index bd0f1e46445..4751aac84ac 100644 --- a/java/pom.xml +++ b/java/pom.xml @@ -411,6 +411,7 @@ maven-compiler-plugin ${maven-compiler-plugin.version} + **/module-info.java false From 06f5805d782be6f436877bdd5a9e770ea2f8a478 Mon Sep 17 00:00:00 2001 From: James Duong Date: Mon, 27 Nov 2023 10:38:19 -0800 Subject: [PATCH 08/30] Exclude module-info.java files from checkstyle Newer versions of the checkstyle plugin do support module-info.java files but require rewriting the checkstyle rules file. --- java/pom.xml | 1 + 1 file changed, 1 insertion(+) diff --git a/java/pom.xml b/java/pom.xml index 4751aac84ac..7dd75528fe6 100644 --- a/java/pom.xml +++ b/java/pom.xml @@ -321,6 +321,7 @@ + **/module-info.java dev/checkstyle/checkstyle.xml dev/checkstyle/checkstyle.license dev/checkstyle/suppressions.xml From 65bb84ed286e0d1cc8a9b11d026d70d016476cca Mon Sep 17 00:00:00 2001 From: James Duong Date: Mon, 27 Nov 2023 12:36:44 -0800 Subject: [PATCH 09/30] Add skip parameter and logging to module info compiler plugin --- .../arrow/maven/plugins/ModuleInfoCompilerPlugin.java | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/java/maven/module-info-compiler-maven-plugin/src/main/java/org/apache/arrow/maven/plugins/ModuleInfoCompilerPlugin.java b/java/maven/module-info-compiler-maven-plugin/src/main/java/org/apache/arrow/maven/plugins/ModuleInfoCompilerPlugin.java index e0475c27ee0..0f91d0590bb 100644 --- a/java/maven/module-info-compiler-maven-plugin/src/main/java/org/apache/arrow/maven/plugins/ModuleInfoCompilerPlugin.java +++ b/java/maven/module-info-compiler-maven-plugin/src/main/java/org/apache/arrow/maven/plugins/ModuleInfoCompilerPlugin.java @@ -50,11 +50,19 @@ public class ModuleInfoCompilerPlugin extends AbstractMojo { required = true) private final List compileSourceRoots = new ArrayList<>(); + @Parameter(defaultValue = "false", property = "skip", required = false) + private boolean skip = false; + @Parameter(defaultValue = "${project}", readonly = true, required = true) private MavenProject project; @Override public void execute() throws MojoExecutionException { + if (skip) { + getLog().info("Skipping module-info-compiler-maven-plugin"); + return; + } + Optional moduleInfoFile = findFirstModuleInfo(compileSourceRoots); if (moduleInfoFile.isPresent()) { // The compiled module-info.class file goes into target/classes/module-info/main @@ -69,9 +77,12 @@ public void execute() throws MojoExecutionException { StandardCharsets.UTF_8); OutputStream output = Files.newOutputStream(targetPath)) { compiler.compile(reader, output); + getLog().info("Successfully wrote module-info.class file."); } catch (IOException ex) { throw new MojoExecutionException("Error compiling module-info.java", ex); } + } else { + getLog().info("No module-info.java file found. module-info.class file was not generated."); } } From 807ee9a095c5578ea4ecb05705a65c40cc27b600 Mon Sep 17 00:00:00 2001 From: James Duong Date: Tue, 28 Nov 2023 00:12:32 -0800 Subject: [PATCH 10/30] Add module-info.java files for format and vector --- java/format/src/main/java/module-info.java | 21 ++++++++++++++++ java/vector/src/main/java/module-info.java | 29 ++++++++++++++++++++++ 2 files changed, 50 insertions(+) create mode 100644 java/format/src/main/java/module-info.java create mode 100644 java/vector/src/main/java/module-info.java diff --git a/java/format/src/main/java/module-info.java b/java/format/src/main/java/module-info.java new file mode 100644 index 00000000000..bda779c91af --- /dev/null +++ b/java/format/src/main/java/module-info.java @@ -0,0 +1,21 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +module org.apache.arrow.format { + exports org.apache.arrow.flatbuf; + requires transitive flatbuffers.java; +} diff --git a/java/vector/src/main/java/module-info.java b/java/vector/src/main/java/module-info.java new file mode 100644 index 00000000000..9ca1d43985d --- /dev/null +++ b/java/vector/src/main/java/module-info.java @@ -0,0 +1,29 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +module org.apache.arrow.vector { + requires com.fasterxml.jackson.annotation; + requires com.fasterxml.jackson.core; + requires com.fasterxml.jackson.databind; + requires com.fasterxml.jackson.datatype.jsr310; + requires flatbuffers.java; + requires io.netty.common; + requires java.sql; + requires org.apache.arrow.format; + requires org.apache.commons.codec; + requires slf4j.api; +} From ce200981197fd91a0f96b2b0a0cc8d74893a4442 Mon Sep 17 00:00:00 2001 From: James Duong Date: Tue, 28 Nov 2023 12:39:22 -0800 Subject: [PATCH 11/30] Move arrow-vector tests to use a different package Do not add tests in org.apache.arrow.util because that is an exported package for arrow-memory-core and causes module conflicts --- .../src/test/java/org/apache/arrow/flight/FlightTestUtil.java | 2 +- .../org/apache/arrow/{ => vector}/util/ArrowTestDataUtil.java | 2 +- .../java/org/apache/arrow/{ => vector}/util/TestSchemaUtil.java | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) rename java/vector/src/test/java/org/apache/arrow/{ => vector}/util/ArrowTestDataUtil.java (97%) rename java/vector/src/test/java/org/apache/arrow/{ => vector}/util/TestSchemaUtil.java (98%) diff --git a/java/flight/flight-core/src/test/java/org/apache/arrow/flight/FlightTestUtil.java b/java/flight/flight-core/src/test/java/org/apache/arrow/flight/FlightTestUtil.java index 25d59d99ad7..91d7a97ceb4 100644 --- a/java/flight/flight-core/src/test/java/org/apache/arrow/flight/FlightTestUtil.java +++ b/java/flight/flight-core/src/test/java/org/apache/arrow/flight/FlightTestUtil.java @@ -24,7 +24,7 @@ import java.util.List; import java.util.Random; -import org.apache.arrow.util.ArrowTestDataUtil; +import org.apache.arrow.vector.util.ArrowTestDataUtil; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.function.Executable; diff --git a/java/vector/src/test/java/org/apache/arrow/util/ArrowTestDataUtil.java b/java/vector/src/test/java/org/apache/arrow/vector/util/ArrowTestDataUtil.java similarity index 97% rename from java/vector/src/test/java/org/apache/arrow/util/ArrowTestDataUtil.java rename to java/vector/src/test/java/org/apache/arrow/vector/util/ArrowTestDataUtil.java index 120c0adc884..1c525c0c271 100644 --- a/java/vector/src/test/java/org/apache/arrow/util/ArrowTestDataUtil.java +++ b/java/vector/src/test/java/org/apache/arrow/vector/util/ArrowTestDataUtil.java @@ -15,7 +15,7 @@ * limitations under the License. */ -package org.apache.arrow.util; +package org.apache.arrow.vector.util; import java.nio.file.Path; import java.nio.file.Paths; diff --git a/java/vector/src/test/java/org/apache/arrow/util/TestSchemaUtil.java b/java/vector/src/test/java/org/apache/arrow/vector/util/TestSchemaUtil.java similarity index 98% rename from java/vector/src/test/java/org/apache/arrow/util/TestSchemaUtil.java rename to java/vector/src/test/java/org/apache/arrow/vector/util/TestSchemaUtil.java index cefff838232..52b65840868 100644 --- a/java/vector/src/test/java/org/apache/arrow/util/TestSchemaUtil.java +++ b/java/vector/src/test/java/org/apache/arrow/vector/util/TestSchemaUtil.java @@ -15,7 +15,7 @@ * limitations under the License. */ -package org.apache.arrow.util; +package org.apache.arrow.vector.util; import static java.util.Arrays.asList; import static org.junit.Assert.assertEquals; From ba88814b364a6d4bec0e81982770ee360f9af8a1 Mon Sep 17 00:00:00 2001 From: James Duong Date: Tue, 28 Nov 2023 12:40:18 -0800 Subject: [PATCH 12/30] Explicitly have arrow-vector depend on immutables Having an implicit dependency from arrow-memory-core does not put immutables on the module-path when running tests and causes module issues. --- java/vector/pom.xml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/java/vector/pom.xml b/java/vector/pom.xml index 17d8f312a52..a4292449c9c 100644 --- a/java/vector/pom.xml +++ b/java/vector/pom.xml @@ -30,6 +30,10 @@ org.apache.arrow arrow-memory-core + + org.immutables + value + com.fasterxml.jackson.core jackson-core From e14c3cd9679341a86e01b271121172b55e313887 Mon Sep 17 00:00:00 2001 From: James Duong Date: Tue, 28 Nov 2023 12:41:04 -0800 Subject: [PATCH 13/30] Export packages for arrow-vector module Export public package for arrow-vector module. Allow jackson to use arrow-vector pojo classes reflectively for serialization. --- java/vector/src/main/java/module-info.java | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/java/vector/src/main/java/module-info.java b/java/vector/src/main/java/module-info.java index 9ca1d43985d..a64d1caa4d8 100644 --- a/java/vector/src/main/java/module-info.java +++ b/java/vector/src/main/java/module-info.java @@ -16,6 +16,26 @@ */ module org.apache.arrow.vector { + exports org.apache.arrow.vector; + exports org.apache.arrow.vector.compare; + exports org.apache.arrow.vector.compare.util; + exports org.apache.arrow.vector.complex; + exports org.apache.arrow.vector.complex.impl; + exports org.apache.arrow.vector.complex.reader; + exports org.apache.arrow.vector.complex.writer; + exports org.apache.arrow.vector.compression; + exports org.apache.arrow.vector.dictionary; + exports org.apache.arrow.vector.holders; + exports org.apache.arrow.vector.ipc; + exports org.apache.arrow.vector.ipc.message; + exports org.apache.arrow.vector.table; + exports org.apache.arrow.vector.types; + exports org.apache.arrow.vector.types.pojo; + exports org.apache.arrow.vector.util; + exports org.apache.arrow.vector.validate; + + opens org.apache.arrow.vector.types.pojo to com.fasterxml.jackson.databind; + requires com.fasterxml.jackson.annotation; requires com.fasterxml.jackson.core; requires com.fasterxml.jackson.databind; From 8484c63a15deb417e561764f54c42ac51e95b677 Mon Sep 17 00:00:00 2001 From: James Duong Date: Tue, 28 Nov 2023 14:47:47 -0800 Subject: [PATCH 14/30] Fix a missed reference to moved arrow-vector test classes --- .../src/test/java/org/apache/arrow/dataset/TestAllTypes.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/dataset/src/test/java/org/apache/arrow/dataset/TestAllTypes.java b/java/dataset/src/test/java/org/apache/arrow/dataset/TestAllTypes.java index 7be49079e74..5293aca0c32 100644 --- a/java/dataset/src/test/java/org/apache/arrow/dataset/TestAllTypes.java +++ b/java/dataset/src/test/java/org/apache/arrow/dataset/TestAllTypes.java @@ -32,7 +32,6 @@ import org.apache.arrow.dataset.file.DatasetFileWriter; import org.apache.arrow.dataset.file.FileFormat; import org.apache.arrow.memory.BufferAllocator; -import org.apache.arrow.util.ArrowTestDataUtil; import org.apache.arrow.vector.BigIntVector; import org.apache.arrow.vector.BitVector; import org.apache.arrow.vector.DateMilliVector; @@ -76,6 +75,7 @@ import org.apache.arrow.vector.types.pojo.Field; import org.apache.arrow.vector.types.pojo.FieldType; import org.apache.arrow.vector.types.pojo.Schema; +import org.apache.arrow.vector.util.ArrowTestDataUtil; import org.apache.arrow.vector.util.ByteArrayReadableSeekableByteChannel; import org.apache.arrow.vector.util.Text; import org.junit.ClassRule; From 869d85a2ecba3978291825ffe42cbaa54a341c46 Mon Sep 17 00:00:00 2001 From: James Duong Date: Thu, 23 Nov 2023 12:31:40 -0800 Subject: [PATCH 15/30] Fix CData related test failures --- java/c/pom.xml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/java/c/pom.xml b/java/c/pom.xml index d66379d3566..8fc3f36994d 100644 --- a/java/c/pom.xml +++ b/java/c/pom.xml @@ -53,6 +53,11 @@ arrow-memory-unsafe test + + org.apache.arrow + arrow-format + test + com.google.guava guava From 83bab66d112aeb00f76fbfe5d1f3cfc101201235 Mon Sep 17 00:00:00 2001 From: James Duong Date: Wed, 29 Nov 2023 18:31:48 -0800 Subject: [PATCH 16/30] Have arrow-vector depend on jdk.unsupported Needed because Vector classes use MemoryUtil.UNSAFE. --- java/vector/src/main/java/module-info.java | 1 + 1 file changed, 1 insertion(+) diff --git a/java/vector/src/main/java/module-info.java b/java/vector/src/main/java/module-info.java index a64d1caa4d8..346e809b854 100644 --- a/java/vector/src/main/java/module-info.java +++ b/java/vector/src/main/java/module-info.java @@ -43,6 +43,7 @@ requires flatbuffers.java; requires io.netty.common; requires java.sql; + requires jdk.unsupported; requires org.apache.arrow.format; requires org.apache.commons.codec; requires slf4j.api; From 04b491284a0b369acc5cac67f9df45ddf8bb77dc Mon Sep 17 00:00:00 2001 From: James Duong Date: Fri, 24 Nov 2023 06:30:46 -0800 Subject: [PATCH 17/30] Refactor memory modules for JPMS support Based on #13072 - Avoid having multiple memory modules contribute to the same package - Introduce memory-netty-buffer-patch module for patching classes into Netty modules - Avoid using BaseAllocator in tests and use RootAllocator instead - Move TestBaseAllocator#testMemoryUsage() to a new test in memory-netty TestNettyAllocator because TestBaseAllocator is now in memory-core, but that specific test has Netty dependencies. --- .../apache/arrow/memory/CheckAllocator.java | 39 ++++- .../DefaultAllocationManagerOption.java | 13 +- .../memory/CountingAllocationListener.java | 4 + .../arrow/memory/TestBaseAllocator.java | 134 ++++++------------ .../arrow/memory/TestForeignAllocation.java | 7 + java/memory/memory-netty-buffer-patch/pom.xml | 44 ++++++ .../io/netty/buffer/ExpandableByteBuf.java | 0 .../java/io/netty/buffer/LargeBuffer.java | 0 .../netty/buffer/MutableWrappedByteBuf.java | 0 .../java/io/netty/buffer/NettyArrowBuf.java | 16 ++- .../netty/buffer/PooledByteBufAllocatorL.java | 7 +- .../buffer/UnsafeDirectLittleEndian.java | 0 .../memory/patch}/ArrowByteBufAllocator.java | 4 +- .../buffer/TestUnsafeDirectLittleEndian.java | 5 + java/memory/memory-netty/pom.xml | 7 + .../DefaultAllocationManagerFactory.java | 6 +- .../{ => netty}/NettyAllocationManager.java | 7 +- .../netty/buffer/TestExpandableByteBuf.java | 4 + .../io/netty/buffer/TestNettyArrowBuf.java | 5 +- .../{ => netty}/ITTestLargeArrowBuf.java | 5 +- .../TestAllocationManagerNetty.java | 4 +- .../memory/{ => netty}/TestEmptyArrowBuf.java | 9 +- .../memory/{ => netty}/TestEndianness.java | 4 +- .../TestNettyAllocationManager.java | 15 +- .../memory/netty/TestNettyAllocator.java | 75 ++++++++++ .../DefaultAllocationManagerFactory.java | 6 +- .../{ => unsafe}/UnsafeAllocationManager.java | 6 +- .../TestAllocationManagerUnsafe.java | 4 +- .../TestUnsafeAllocationManager.java | 13 +- java/memory/pom.xml | 1 + 30 files changed, 314 insertions(+), 130 deletions(-) rename java/memory/{memory-netty => memory-core}/src/test/java/org/apache/arrow/memory/CountingAllocationListener.java (95%) rename java/memory/{memory-netty => memory-core}/src/test/java/org/apache/arrow/memory/TestBaseAllocator.java (91%) rename java/memory/{memory-netty => memory-core}/src/test/java/org/apache/arrow/memory/TestForeignAllocation.java (93%) create mode 100644 java/memory/memory-netty-buffer-patch/pom.xml rename java/memory/{memory-netty => memory-netty-buffer-patch}/src/main/java/io/netty/buffer/ExpandableByteBuf.java (100%) rename java/memory/{memory-netty => memory-netty-buffer-patch}/src/main/java/io/netty/buffer/LargeBuffer.java (100%) rename java/memory/{memory-netty => memory-netty-buffer-patch}/src/main/java/io/netty/buffer/MutableWrappedByteBuf.java (100%) rename java/memory/{memory-netty => memory-netty-buffer-patch}/src/main/java/io/netty/buffer/NettyArrowBuf.java (96%) rename java/memory/{memory-netty => memory-netty-buffer-patch}/src/main/java/io/netty/buffer/PooledByteBufAllocatorL.java (97%) rename java/memory/{memory-netty => memory-netty-buffer-patch}/src/main/java/io/netty/buffer/UnsafeDirectLittleEndian.java (100%) rename java/memory/{memory-netty/src/main/java/org/apache/arrow/memory => memory-netty-buffer-patch/src/main/java/org/apache/arrow/memory/patch}/ArrowByteBufAllocator.java (97%) rename java/memory/{memory-netty => memory-netty-buffer-patch}/src/test/java/io/netty/buffer/TestUnsafeDirectLittleEndian.java (95%) rename java/memory/memory-netty/src/main/java/org/apache/arrow/memory/{ => netty}/DefaultAllocationManagerFactory.java (87%) rename java/memory/memory-netty/src/main/java/org/apache/arrow/memory/{ => netty}/NettyAllocationManager.java (94%) rename java/memory/memory-netty/src/test/java/org/apache/arrow/memory/{ => netty}/ITTestLargeArrowBuf.java (93%) rename java/memory/memory-netty/src/test/java/org/apache/arrow/memory/{ => netty}/TestAllocationManagerNetty.java (90%) rename java/memory/memory-netty/src/test/java/org/apache/arrow/memory/{ => netty}/TestEmptyArrowBuf.java (90%) rename java/memory/memory-netty/src/test/java/org/apache/arrow/memory/{ => netty}/TestEndianness.java (92%) rename java/memory/memory-netty/src/test/java/org/apache/arrow/memory/{ => netty}/TestNettyAllocationManager.java (87%) create mode 100644 java/memory/memory-netty/src/test/java/org/apache/arrow/memory/netty/TestNettyAllocator.java rename java/memory/memory-unsafe/src/main/java/org/apache/arrow/memory/{ => unsafe}/DefaultAllocationManagerFactory.java (87%) rename java/memory/memory-unsafe/src/main/java/org/apache/arrow/memory/{ => unsafe}/UnsafeAllocationManager.java (89%) rename java/memory/memory-unsafe/src/test/java/org/apache/arrow/memory/{ => unsafe}/TestAllocationManagerUnsafe.java (90%) rename java/memory/memory-unsafe/src/test/java/org/apache/arrow/memory/{ => unsafe}/TestUnsafeAllocationManager.java (82%) diff --git a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/CheckAllocator.java b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/CheckAllocator.java index 79b825aa2e8..dac4a3fcff5 100644 --- a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/CheckAllocator.java +++ b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/CheckAllocator.java @@ -31,20 +31,35 @@ */ final class CheckAllocator { private static final Logger logger = LoggerFactory.getLogger(CheckAllocator.class); - private static final String ALLOCATOR_PATH = "org/apache/arrow/memory/DefaultAllocationManagerFactory.class"; + // unique package names needed by JPMS module naming + private static final String ALLOCATOR_PATH_CORE = + "org/apache/arrow/memory/DefaultAllocationManagerFactory.class"; + private static final String ALLOCATOR_PATH_UNSAFE = + "org/apache/arrow/memory/unsafe/DefaultAllocationManagerFactory.class"; + private static final String ALLOCATOR_PATH_NETTY = + "org/apache/arrow/memory/netty/DefaultAllocationManagerFactory.class"; private CheckAllocator() { - } static String check() { Set urls = scanClasspath(); URL rootAllocator = assertOnlyOne(urls); reportResult(rootAllocator); - return "org.apache.arrow.memory.DefaultAllocationManagerFactory"; + if (rootAllocator.getPath().contains("memory-core") || + rootAllocator.getPath().contains("/org/apache/arrow/memory/core/")) { + return "org.apache.arrow.memory.DefaultAllocationManagerFactory"; + } else if (rootAllocator.getPath().contains("memory-unsafe") || + rootAllocator.getPath().contains("/org/apache/arrow/memory/unsafe/")) { + return "org.apache.arrow.memory.unsafe.DefaultAllocationManagerFactory"; + } else if (rootAllocator.getPath().contains("memory-netty") || + rootAllocator.getPath().contains("/org/apache/arrow/memory/netty/")) { + return "org.apache.arrow.memory.netty.DefaultAllocationManagerFactory"; + } else { + throw new IllegalStateException("Unknown allocation manager type to infer. Current: " + rootAllocator.getPath()); + } } - private static Set scanClasspath() { // LinkedHashSet appropriate here because it preserves insertion order // during iteration @@ -53,9 +68,21 @@ private static Set scanClasspath() { ClassLoader allocatorClassLoader = CheckAllocator.class.getClassLoader(); Enumeration paths; if (allocatorClassLoader == null) { - paths = ClassLoader.getSystemResources(ALLOCATOR_PATH); + paths = ClassLoader.getSystemResources(ALLOCATOR_PATH_CORE); + if (!paths.hasMoreElements()) { + paths = ClassLoader.getSystemResources(ALLOCATOR_PATH_UNSAFE); + } + if (!paths.hasMoreElements()) { + paths = ClassLoader.getSystemResources(ALLOCATOR_PATH_NETTY); + } } else { - paths = allocatorClassLoader.getResources(ALLOCATOR_PATH); + paths = allocatorClassLoader.getResources(ALLOCATOR_PATH_CORE); + if (!paths.hasMoreElements()) { + paths = allocatorClassLoader.getResources(ALLOCATOR_PATH_UNSAFE); + } + if (!paths.hasMoreElements()) { + paths = allocatorClassLoader.getResources(ALLOCATOR_PATH_NETTY); + } } while (paths.hasMoreElements()) { URL path = paths.nextElement(); diff --git a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/DefaultAllocationManagerOption.java b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/DefaultAllocationManagerOption.java index 15120c252fc..564d59994b7 100644 --- a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/DefaultAllocationManagerOption.java +++ b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/DefaultAllocationManagerOption.java @@ -19,6 +19,8 @@ import java.lang.reflect.Field; +import org.apache.arrow.util.VisibleForTesting; + /** * A class for choosing the default allocation manager. */ @@ -61,7 +63,12 @@ public enum AllocationManagerType { Unknown, } - static AllocationManagerType getDefaultAllocationManagerType() { + /** + * Returns the default allocation manager type. + * @return the default allocation manager type. + */ + @VisibleForTesting + public static AllocationManagerType getDefaultAllocationManagerType() { AllocationManagerType ret = AllocationManagerType.Unknown; try { @@ -115,7 +122,7 @@ private static AllocationManager.Factory getFactory(String clazzName) { private static AllocationManager.Factory getUnsafeFactory() { try { - return getFactory("org.apache.arrow.memory.UnsafeAllocationManager"); + return getFactory("org.apache.arrow.memory.unsafe.UnsafeAllocationManager"); } catch (RuntimeException e) { throw new RuntimeException("Please add arrow-memory-unsafe to your classpath," + " No DefaultAllocationManager found to instantiate an UnsafeAllocationManager", e); @@ -124,7 +131,7 @@ private static AllocationManager.Factory getUnsafeFactory() { private static AllocationManager.Factory getNettyFactory() { try { - return getFactory("org.apache.arrow.memory.NettyAllocationManager"); + return getFactory("org.apache.arrow.memory.netty.NettyAllocationManager"); } catch (RuntimeException e) { throw new RuntimeException("Please add arrow-memory-netty to your classpath," + " No DefaultAllocationManager found to instantiate an NettyAllocationManager", e); diff --git a/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/CountingAllocationListener.java b/java/memory/memory-core/src/test/java/org/apache/arrow/memory/CountingAllocationListener.java similarity index 95% rename from java/memory/memory-netty/src/test/java/org/apache/arrow/memory/CountingAllocationListener.java rename to java/memory/memory-core/src/test/java/org/apache/arrow/memory/CountingAllocationListener.java index 78c78c8ad8c..f1dd7e92c5c 100644 --- a/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/CountingAllocationListener.java +++ b/java/memory/memory-core/src/test/java/org/apache/arrow/memory/CountingAllocationListener.java @@ -17,6 +17,10 @@ package org.apache.arrow.memory; +import org.apache.arrow.memory.AllocationListener; +import org.apache.arrow.memory.AllocationOutcome; +import org.apache.arrow.memory.BufferAllocator; + /** * Counting allocation listener. * It counts the number of times it has been invoked, and how much memory allocation it has seen diff --git a/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestBaseAllocator.java b/java/memory/memory-core/src/test/java/org/apache/arrow/memory/TestBaseAllocator.java similarity index 91% rename from java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestBaseAllocator.java rename to java/memory/memory-core/src/test/java/org/apache/arrow/memory/TestBaseAllocator.java index 7613d073f8c..365c84f5a2b 100644 --- a/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestBaseAllocator.java +++ b/java/memory/memory-core/src/test/java/org/apache/arrow/memory/TestBaseAllocator.java @@ -31,22 +31,16 @@ import java.util.Collection; import java.util.Collections; import java.util.Iterator; -import java.util.stream.Collectors; import org.apache.arrow.memory.AllocationOutcomeDetails.Entry; import org.apache.arrow.memory.rounding.RoundingPolicy; import org.apache.arrow.memory.rounding.SegmentRoundingPolicy; import org.apache.arrow.memory.util.AssertionUtil; +import org.junit.Assert; import org.junit.Ignore; import org.junit.Test; import org.junit.jupiter.api.Assertions; -import org.slf4j.LoggerFactory; -import ch.qos.logback.classic.Level; -import ch.qos.logback.classic.Logger; -import ch.qos.logback.classic.spi.ILoggingEvent; -import ch.qos.logback.core.read.ListAppender; -import io.netty.buffer.PooledByteBufAllocatorL; import sun.misc.Unsafe; public class TestBaseAllocator { @@ -448,73 +442,73 @@ public ArrowBuf empty() { @Test public void testRootAllocator_listeners() throws Exception { CountingAllocationListener l1 = new CountingAllocationListener(); - assertEquals(0, l1.getNumPreCalls()); - assertEquals(0, l1.getNumCalls()); - assertEquals(0, l1.getNumReleaseCalls()); - assertEquals(0, l1.getNumChildren()); - assertEquals(0, l1.getTotalMem()); + Assert.assertEquals(0, l1.getNumPreCalls()); + Assert.assertEquals(0, l1.getNumCalls()); + Assert.assertEquals(0, l1.getNumReleaseCalls()); + Assert.assertEquals(0, l1.getNumChildren()); + Assert.assertEquals(0, l1.getTotalMem()); CountingAllocationListener l2 = new CountingAllocationListener(); - assertEquals(0, l2.getNumPreCalls()); - assertEquals(0, l2.getNumCalls()); - assertEquals(0, l2.getNumReleaseCalls()); - assertEquals(0, l2.getNumChildren()); - assertEquals(0, l2.getTotalMem()); + Assert.assertEquals(0, l2.getNumPreCalls()); + Assert.assertEquals(0, l2.getNumCalls()); + Assert.assertEquals(0, l2.getNumReleaseCalls()); + Assert.assertEquals(0, l2.getNumChildren()); + Assert.assertEquals(0, l2.getTotalMem()); // root and first-level child share the first listener // second-level and third-level child share the second listener try (final RootAllocator rootAllocator = new RootAllocator(l1, MAX_ALLOCATION)) { try (final BufferAllocator c1 = rootAllocator.newChildAllocator("c1", 0, MAX_ALLOCATION)) { - assertEquals(1, l1.getNumChildren()); + Assert.assertEquals(1, l1.getNumChildren()); final ArrowBuf buf1 = c1.buffer(16); assertNotNull("allocation failed", buf1); - assertEquals(1, l1.getNumPreCalls()); - assertEquals(1, l1.getNumCalls()); - assertEquals(0, l1.getNumReleaseCalls()); - assertEquals(16, l1.getTotalMem()); + Assert.assertEquals(1, l1.getNumPreCalls()); + Assert.assertEquals(1, l1.getNumCalls()); + Assert.assertEquals(0, l1.getNumReleaseCalls()); + Assert.assertEquals(16, l1.getTotalMem()); buf1.getReferenceManager().release(); try (final BufferAllocator c2 = c1.newChildAllocator("c2", l2, 0, MAX_ALLOCATION)) { - assertEquals(2, l1.getNumChildren()); // c1 got a new child, so c1's listener (l1) is notified - assertEquals(0, l2.getNumChildren()); + Assert.assertEquals(2, l1.getNumChildren()); // c1 got a new child, so c1's listener (l1) is notified + Assert.assertEquals(0, l2.getNumChildren()); final ArrowBuf buf2 = c2.buffer(32); assertNotNull("allocation failed", buf2); - assertEquals(1, l1.getNumCalls()); - assertEquals(16, l1.getTotalMem()); - assertEquals(1, l2.getNumPreCalls()); - assertEquals(1, l2.getNumCalls()); - assertEquals(0, l2.getNumReleaseCalls()); - assertEquals(32, l2.getTotalMem()); + Assert.assertEquals(1, l1.getNumCalls()); + Assert.assertEquals(16, l1.getTotalMem()); + Assert.assertEquals(1, l2.getNumPreCalls()); + Assert.assertEquals(1, l2.getNumCalls()); + Assert.assertEquals(0, l2.getNumReleaseCalls()); + Assert.assertEquals(32, l2.getTotalMem()); buf2.getReferenceManager().release(); try (final BufferAllocator c3 = c2.newChildAllocator("c3", 0, MAX_ALLOCATION)) { - assertEquals(2, l1.getNumChildren()); - assertEquals(1, l2.getNumChildren()); + Assert.assertEquals(2, l1.getNumChildren()); + Assert.assertEquals(1, l2.getNumChildren()); final ArrowBuf buf3 = c3.buffer(64); assertNotNull("allocation failed", buf3); - assertEquals(1, l1.getNumPreCalls()); - assertEquals(1, l1.getNumCalls()); - assertEquals(1, l1.getNumReleaseCalls()); - assertEquals(16, l1.getTotalMem()); - assertEquals(2, l2.getNumPreCalls()); - assertEquals(2, l2.getNumCalls()); - assertEquals(1, l2.getNumReleaseCalls()); - assertEquals(32 + 64, l2.getTotalMem()); + Assert.assertEquals(1, l1.getNumPreCalls()); + Assert.assertEquals(1, l1.getNumCalls()); + Assert.assertEquals(1, l1.getNumReleaseCalls()); + Assert.assertEquals(16, l1.getTotalMem()); + Assert.assertEquals(2, l2.getNumPreCalls()); + Assert.assertEquals(2, l2.getNumCalls()); + Assert.assertEquals(1, l2.getNumReleaseCalls()); + Assert.assertEquals(32 + 64, l2.getTotalMem()); buf3.getReferenceManager().release(); } - assertEquals(2, l1.getNumChildren()); - assertEquals(0, l2.getNumChildren()); // third-level child removed + Assert.assertEquals(2, l1.getNumChildren()); + Assert.assertEquals(0, l2.getNumChildren()); // third-level child removed } - assertEquals(1, l1.getNumChildren()); // second-level child removed - assertEquals(0, l2.getNumChildren()); + Assert.assertEquals(1, l1.getNumChildren()); // second-level child removed + Assert.assertEquals(0, l2.getNumChildren()); } - assertEquals(0, l1.getNumChildren()); // first-level child removed + Assert.assertEquals(0, l1.getNumChildren()); // first-level child removed - assertEquals(2, l2.getNumReleaseCalls()); + Assert.assertEquals(2, l2.getNumReleaseCalls()); } } @Test public void testRootAllocator_listenerAllocationFail() throws Exception { CountingAllocationListener l1 = new CountingAllocationListener(); - assertEquals(0, l1.getNumCalls()); - assertEquals(0, l1.getTotalMem()); + Assert.assertEquals(0, l1.getNumCalls()); + Assert.assertEquals(0, l1.getTotalMem()); // Test attempts to allocate too much from a child whose limit is set to half of the max // allocation. The listener's callback triggers, expanding the child allocator's limit, so then // the allocation succeeds. @@ -527,14 +521,14 @@ public void testRootAllocator_listenerAllocationFail() throws Exception { } catch (OutOfMemoryException e) { // expected } - assertEquals(0, l1.getNumCalls()); - assertEquals(0, l1.getTotalMem()); + Assert.assertEquals(0, l1.getNumCalls()); + Assert.assertEquals(0, l1.getTotalMem()); l1.setExpandOnFail(c1, MAX_ALLOCATION); ArrowBuf arrowBuf = c1.buffer(MAX_ALLOCATION); assertNotNull("allocation failed", arrowBuf); - assertEquals(1, l1.getNumCalls()); - assertEquals(MAX_ALLOCATION, l1.getTotalMem()); + Assert.assertEquals(1, l1.getNumCalls()); + Assert.assertEquals(MAX_ALLOCATION, l1.getTotalMem()); arrowBuf.getReferenceManager().release(); } } @@ -1098,42 +1092,6 @@ public void testMemoryLeakWithReservation() throws Exception { } } - @Test - public void testMemoryUsage() { - ListAppender memoryLogsAppender = new ListAppender<>(); - Logger logger = (Logger) LoggerFactory.getLogger("arrow.allocator"); - try { - logger.setLevel(Level.TRACE); - logger.addAppender(memoryLogsAppender); - memoryLogsAppender.start(); - try (ArrowBuf buf = new ArrowBuf(ReferenceManager.NO_OP, null, - 1024, new PooledByteBufAllocatorL().empty.memoryAddress())) { - buf.memoryAddress(); - } - boolean result = false; - long startTime = System.currentTimeMillis(); - while ((System.currentTimeMillis() - startTime) < 10000) { // 10 seconds maximum for time to read logs - result = memoryLogsAppender.list.stream() - .anyMatch( - log -> log.toString().contains("Memory Usage: \n") && - log.toString().contains("Large buffers outstanding: ") && - log.toString().contains("Normal buffers outstanding: ") && - log.getLevel().equals(Level.TRACE) - ); - if (result) { - break; - } - } - assertTrue("Log messages are:\n" + - memoryLogsAppender.list.stream().map(ILoggingEvent::toString).collect(Collectors.joining("\n")), - result); - } finally { - memoryLogsAppender.stop(); - logger.detachAppender(memoryLogsAppender); - logger.setLevel(null); - } - } - @Test public void testOverlimit() { try (BufferAllocator allocator = new RootAllocator(1024)) { diff --git a/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestForeignAllocation.java b/java/memory/memory-core/src/test/java/org/apache/arrow/memory/TestForeignAllocation.java similarity index 93% rename from java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestForeignAllocation.java rename to java/memory/memory-core/src/test/java/org/apache/arrow/memory/TestForeignAllocation.java index ec049ca6923..46e94fad37b 100644 --- a/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestForeignAllocation.java +++ b/java/memory/memory-core/src/test/java/org/apache/arrow/memory/TestForeignAllocation.java @@ -23,6 +23,13 @@ import java.util.ArrayList; import java.util.List; +import org.apache.arrow.memory.AllocationListener; +import org.apache.arrow.memory.AllocationOutcome; +import org.apache.arrow.memory.ArrowBuf; +import org.apache.arrow.memory.BufferAllocator; +import org.apache.arrow.memory.ForeignAllocation; +import org.apache.arrow.memory.OutOfMemoryException; +import org.apache.arrow.memory.RootAllocator; import org.apache.arrow.memory.util.MemoryUtil; import org.junit.After; import org.junit.Before; diff --git a/java/memory/memory-netty-buffer-patch/pom.xml b/java/memory/memory-netty-buffer-patch/pom.xml new file mode 100644 index 00000000000..1d4407c638d --- /dev/null +++ b/java/memory/memory-netty-buffer-patch/pom.xml @@ -0,0 +1,44 @@ + + + + + arrow-memory + org.apache.arrow + 15.0.0-SNAPSHOT + + 4.0.0 + + arrow-memory-netty-buffer-patch + Arrow Memory - Netty Buffer + Netty Buffer needed to patch that is consumed by Arrow Memory Netty + + + + org.apache.arrow + arrow-memory-core + + + io.netty + netty-buffer + + + io.netty + netty-common + + + org.slf4j + slf4j-api + + + diff --git a/java/memory/memory-netty/src/main/java/io/netty/buffer/ExpandableByteBuf.java b/java/memory/memory-netty-buffer-patch/src/main/java/io/netty/buffer/ExpandableByteBuf.java similarity index 100% rename from java/memory/memory-netty/src/main/java/io/netty/buffer/ExpandableByteBuf.java rename to java/memory/memory-netty-buffer-patch/src/main/java/io/netty/buffer/ExpandableByteBuf.java diff --git a/java/memory/memory-netty/src/main/java/io/netty/buffer/LargeBuffer.java b/java/memory/memory-netty-buffer-patch/src/main/java/io/netty/buffer/LargeBuffer.java similarity index 100% rename from java/memory/memory-netty/src/main/java/io/netty/buffer/LargeBuffer.java rename to java/memory/memory-netty-buffer-patch/src/main/java/io/netty/buffer/LargeBuffer.java diff --git a/java/memory/memory-netty/src/main/java/io/netty/buffer/MutableWrappedByteBuf.java b/java/memory/memory-netty-buffer-patch/src/main/java/io/netty/buffer/MutableWrappedByteBuf.java similarity index 100% rename from java/memory/memory-netty/src/main/java/io/netty/buffer/MutableWrappedByteBuf.java rename to java/memory/memory-netty-buffer-patch/src/main/java/io/netty/buffer/MutableWrappedByteBuf.java diff --git a/java/memory/memory-netty/src/main/java/io/netty/buffer/NettyArrowBuf.java b/java/memory/memory-netty-buffer-patch/src/main/java/io/netty/buffer/NettyArrowBuf.java similarity index 96% rename from java/memory/memory-netty/src/main/java/io/netty/buffer/NettyArrowBuf.java rename to java/memory/memory-netty-buffer-patch/src/main/java/io/netty/buffer/NettyArrowBuf.java index 71e4b7cb6d5..466444c7d53 100644 --- a/java/memory/memory-netty/src/main/java/io/netty/buffer/NettyArrowBuf.java +++ b/java/memory/memory-netty-buffer-patch/src/main/java/io/netty/buffer/NettyArrowBuf.java @@ -17,8 +17,6 @@ package io.netty.buffer; -import static org.apache.arrow.memory.util.LargeMemoryUtil.checkedCastToInt; - import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; @@ -29,10 +27,12 @@ import java.nio.channels.ScatteringByteChannel; import org.apache.arrow.memory.ArrowBuf; -import org.apache.arrow.memory.ArrowByteBufAllocator; import org.apache.arrow.memory.BoundsChecking; import org.apache.arrow.memory.BufferAllocator; +import org.apache.arrow.memory.patch.ArrowByteBufAllocator; +import org.apache.arrow.memory.util.LargeMemoryUtil; import org.apache.arrow.util.Preconditions; +import org.apache.arrow.util.VisibleForTesting; import io.netty.util.internal.PlatformDependent; @@ -264,7 +264,7 @@ public ByteBuffer nioBuffer(long index, int length) { * @return ByteBuffer */ private ByteBuffer getDirectBuffer(long index) { - return PlatformDependent.directBuffer(addr(index), checkedCastToInt(length - index)); + return PlatformDependent.directBuffer(addr(index), LargeMemoryUtil.checkedCastToInt(length - index)); } @Override @@ -580,11 +580,13 @@ public NettyArrowBuf setMedium(int index, int value) { } @Override + @VisibleForTesting protected void _setInt(int index, int value) { setInt(index, value); } @Override + @VisibleForTesting protected void _setIntLE(int index, int value) { this.chk(index, 4); PlatformDependent.putInt(this.addr(index), Integer.reverseBytes(value)); @@ -620,9 +622,9 @@ public static NettyArrowBuf unwrapBuffer(ArrowBuf buf) { final NettyArrowBuf nettyArrowBuf = new NettyArrowBuf( buf, buf.getReferenceManager().getAllocator(), - checkedCastToInt(buf.capacity())); - nettyArrowBuf.readerIndex(checkedCastToInt(buf.readerIndex())); - nettyArrowBuf.writerIndex(checkedCastToInt(buf.writerIndex())); + LargeMemoryUtil.checkedCastToInt(buf.capacity())); + nettyArrowBuf.readerIndex(LargeMemoryUtil.checkedCastToInt(buf.readerIndex())); + nettyArrowBuf.writerIndex(LargeMemoryUtil.checkedCastToInt(buf.writerIndex())); return nettyArrowBuf; } diff --git a/java/memory/memory-netty/src/main/java/io/netty/buffer/PooledByteBufAllocatorL.java b/java/memory/memory-netty-buffer-patch/src/main/java/io/netty/buffer/PooledByteBufAllocatorL.java similarity index 97% rename from java/memory/memory-netty/src/main/java/io/netty/buffer/PooledByteBufAllocatorL.java rename to java/memory/memory-netty-buffer-patch/src/main/java/io/netty/buffer/PooledByteBufAllocatorL.java index 06c6669cfd1..bc95b9df545 100644 --- a/java/memory/memory-netty/src/main/java/io/netty/buffer/PooledByteBufAllocatorL.java +++ b/java/memory/memory-netty-buffer-patch/src/main/java/io/netty/buffer/PooledByteBufAllocatorL.java @@ -17,13 +17,12 @@ package io.netty.buffer; -import static org.apache.arrow.memory.util.AssertionUtil.ASSERT_ENABLED; - import java.lang.reflect.Field; import java.nio.ByteBuffer; import java.util.concurrent.atomic.AtomicLong; import org.apache.arrow.memory.OutOfMemoryException; +import org.apache.arrow.memory.util.AssertionUtil; import org.apache.arrow.memory.util.LargeMemoryUtil; import io.netty.util.internal.OutOfDirectMemoryError; @@ -51,7 +50,7 @@ public PooledByteBufAllocatorL() { } /** - * Returns a {@linkplain io.netty.buffer.UnsafeDirectLittleEndian} of the given size. + * Returns a {@linkplain UnsafeDirectLittleEndian} of the given size. */ public UnsafeDirectLittleEndian allocate(long size) { try { @@ -183,7 +182,7 @@ private UnsafeDirectLittleEndian newDirectBufferL(int initialCapacity, int maxCa fail(); } - if (!ASSERT_ENABLED) { + if (!AssertionUtil.ASSERT_ENABLED) { return new UnsafeDirectLittleEndian((PooledUnsafeDirectByteBuf) buf); } diff --git a/java/memory/memory-netty/src/main/java/io/netty/buffer/UnsafeDirectLittleEndian.java b/java/memory/memory-netty-buffer-patch/src/main/java/io/netty/buffer/UnsafeDirectLittleEndian.java similarity index 100% rename from java/memory/memory-netty/src/main/java/io/netty/buffer/UnsafeDirectLittleEndian.java rename to java/memory/memory-netty-buffer-patch/src/main/java/io/netty/buffer/UnsafeDirectLittleEndian.java diff --git a/java/memory/memory-netty/src/main/java/org/apache/arrow/memory/ArrowByteBufAllocator.java b/java/memory/memory-netty-buffer-patch/src/main/java/org/apache/arrow/memory/patch/ArrowByteBufAllocator.java similarity index 97% rename from java/memory/memory-netty/src/main/java/org/apache/arrow/memory/ArrowByteBufAllocator.java rename to java/memory/memory-netty-buffer-patch/src/main/java/org/apache/arrow/memory/patch/ArrowByteBufAllocator.java index ff40b49ff6f..6ce08b5a590 100644 --- a/java/memory/memory-netty/src/main/java/org/apache/arrow/memory/ArrowByteBufAllocator.java +++ b/java/memory/memory-netty-buffer-patch/src/main/java/org/apache/arrow/memory/patch/ArrowByteBufAllocator.java @@ -15,7 +15,9 @@ * limitations under the License. */ -package org.apache.arrow.memory; +package org.apache.arrow.memory.patch; + +import org.apache.arrow.memory.BufferAllocator; import io.netty.buffer.AbstractByteBufAllocator; import io.netty.buffer.ByteBuf; diff --git a/java/memory/memory-netty/src/test/java/io/netty/buffer/TestUnsafeDirectLittleEndian.java b/java/memory/memory-netty-buffer-patch/src/test/java/io/netty/buffer/TestUnsafeDirectLittleEndian.java similarity index 95% rename from java/memory/memory-netty/src/test/java/io/netty/buffer/TestUnsafeDirectLittleEndian.java rename to java/memory/memory-netty-buffer-patch/src/test/java/io/netty/buffer/TestUnsafeDirectLittleEndian.java index c2bd95bb3d9..043c2c1605a 100644 --- a/java/memory/memory-netty/src/test/java/io/netty/buffer/TestUnsafeDirectLittleEndian.java +++ b/java/memory/memory-netty-buffer-patch/src/test/java/io/netty/buffer/TestUnsafeDirectLittleEndian.java @@ -28,6 +28,11 @@ import org.junit.Test; +import io.netty.buffer.ByteBuf; +import io.netty.buffer.LargeBuffer; +import io.netty.buffer.Unpooled; +import io.netty.buffer.UnsafeDirectLittleEndian; + public class TestUnsafeDirectLittleEndian { private static final boolean LITTLE_ENDIAN = ByteOrder.nativeOrder() == ByteOrder.LITTLE_ENDIAN; diff --git a/java/memory/memory-netty/pom.xml b/java/memory/memory-netty/pom.xml index 307f6ad81a0..4ee4883d78d 100644 --- a/java/memory/memory-netty/pom.xml +++ b/java/memory/memory-netty/pom.xml @@ -26,9 +26,15 @@ org.apache.arrow arrow-memory-core + + org.apache.arrow + arrow-memory-netty-buffer-patch + ${project.version} + io.netty netty-buffer + provided io.netty @@ -37,6 +43,7 @@ org.slf4j slf4j-api + test ch.qos.logback diff --git a/java/memory/memory-netty/src/main/java/org/apache/arrow/memory/DefaultAllocationManagerFactory.java b/java/memory/memory-netty/src/main/java/org/apache/arrow/memory/netty/DefaultAllocationManagerFactory.java similarity index 87% rename from java/memory/memory-netty/src/main/java/org/apache/arrow/memory/DefaultAllocationManagerFactory.java rename to java/memory/memory-netty/src/main/java/org/apache/arrow/memory/netty/DefaultAllocationManagerFactory.java index 10cfb5c1648..8ece77178f0 100644 --- a/java/memory/memory-netty/src/main/java/org/apache/arrow/memory/DefaultAllocationManagerFactory.java +++ b/java/memory/memory-netty/src/main/java/org/apache/arrow/memory/netty/DefaultAllocationManagerFactory.java @@ -15,7 +15,11 @@ * limitations under the License. */ -package org.apache.arrow.memory; +package org.apache.arrow.memory.netty; + +import org.apache.arrow.memory.AllocationManager; +import org.apache.arrow.memory.ArrowBuf; +import org.apache.arrow.memory.BufferAllocator; /** * The default Allocation Manager Factory for a module. diff --git a/java/memory/memory-netty/src/main/java/org/apache/arrow/memory/NettyAllocationManager.java b/java/memory/memory-netty/src/main/java/org/apache/arrow/memory/netty/NettyAllocationManager.java similarity index 94% rename from java/memory/memory-netty/src/main/java/org/apache/arrow/memory/NettyAllocationManager.java rename to java/memory/memory-netty/src/main/java/org/apache/arrow/memory/netty/NettyAllocationManager.java index 20004778307..58354d0c2ee 100644 --- a/java/memory/memory-netty/src/main/java/org/apache/arrow/memory/NettyAllocationManager.java +++ b/java/memory/memory-netty/src/main/java/org/apache/arrow/memory/netty/NettyAllocationManager.java @@ -15,7 +15,12 @@ * limitations under the License. */ -package org.apache.arrow.memory; +package org.apache.arrow.memory.netty; + +import org.apache.arrow.memory.AllocationManager; +import org.apache.arrow.memory.ArrowBuf; +import org.apache.arrow.memory.BufferAllocator; +import org.apache.arrow.memory.ReferenceManager; import io.netty.buffer.PooledByteBufAllocatorL; import io.netty.buffer.UnsafeDirectLittleEndian; diff --git a/java/memory/memory-netty/src/test/java/io/netty/buffer/TestExpandableByteBuf.java b/java/memory/memory-netty/src/test/java/io/netty/buffer/TestExpandableByteBuf.java index b39cca8e8e7..67557b65a62 100644 --- a/java/memory/memory-netty/src/test/java/io/netty/buffer/TestExpandableByteBuf.java +++ b/java/memory/memory-netty/src/test/java/io/netty/buffer/TestExpandableByteBuf.java @@ -23,6 +23,10 @@ import org.junit.Assert; import org.junit.Test; +import io.netty.buffer.ByteBuf; +import io.netty.buffer.ExpandableByteBuf; +import io.netty.buffer.NettyArrowBuf; + public class TestExpandableByteBuf { @Test diff --git a/java/memory/memory-netty/src/test/java/io/netty/buffer/TestNettyArrowBuf.java b/java/memory/memory-netty/src/test/java/io/netty/buffer/TestNettyArrowBuf.java index 45d3b41e8a6..f18bccb4c9d 100644 --- a/java/memory/memory-netty/src/test/java/io/netty/buffer/TestNettyArrowBuf.java +++ b/java/memory/memory-netty/src/test/java/io/netty/buffer/TestNettyArrowBuf.java @@ -20,12 +20,15 @@ import java.nio.ByteBuffer; import org.apache.arrow.memory.ArrowBuf; -import org.apache.arrow.memory.ArrowByteBufAllocator; import org.apache.arrow.memory.BufferAllocator; import org.apache.arrow.memory.RootAllocator; +import org.apache.arrow.memory.patch.ArrowByteBufAllocator; import org.junit.Assert; import org.junit.Test; +import io.netty.buffer.CompositeByteBuf; +import io.netty.buffer.NettyArrowBuf; + public class TestNettyArrowBuf { @Test diff --git a/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/ITTestLargeArrowBuf.java b/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/netty/ITTestLargeArrowBuf.java similarity index 93% rename from java/memory/memory-netty/src/test/java/org/apache/arrow/memory/ITTestLargeArrowBuf.java rename to java/memory/memory-netty/src/test/java/org/apache/arrow/memory/netty/ITTestLargeArrowBuf.java index fa8d510e361..71dba73d289 100644 --- a/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/ITTestLargeArrowBuf.java +++ b/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/netty/ITTestLargeArrowBuf.java @@ -15,10 +15,13 @@ * limitations under the License. */ -package org.apache.arrow.memory; +package org.apache.arrow.memory.netty; import static org.junit.Assert.assertEquals; +import org.apache.arrow.memory.ArrowBuf; +import org.apache.arrow.memory.BufferAllocator; +import org.apache.arrow.memory.RootAllocator; import org.junit.Test; import org.slf4j.Logger; import org.slf4j.LoggerFactory; diff --git a/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestAllocationManagerNetty.java b/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/netty/TestAllocationManagerNetty.java similarity index 90% rename from java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestAllocationManagerNetty.java rename to java/memory/memory-netty/src/test/java/org/apache/arrow/memory/netty/TestAllocationManagerNetty.java index 2dbd56480b8..7f1e34ddc5f 100644 --- a/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestAllocationManagerNetty.java +++ b/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/netty/TestAllocationManagerNetty.java @@ -15,10 +15,12 @@ * limitations under the License. */ -package org.apache.arrow.memory; +package org.apache.arrow.memory.netty; import static org.junit.Assert.assertEquals; +import org.apache.arrow.memory.AllocationManager; +import org.apache.arrow.memory.DefaultAllocationManagerOption; import org.junit.Test; /** diff --git a/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestEmptyArrowBuf.java b/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/netty/TestEmptyArrowBuf.java similarity index 90% rename from java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestEmptyArrowBuf.java rename to java/memory/memory-netty/src/test/java/org/apache/arrow/memory/netty/TestEmptyArrowBuf.java index 3fd7ce74aab..b9948083e6f 100644 --- a/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestEmptyArrowBuf.java +++ b/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/netty/TestEmptyArrowBuf.java @@ -15,10 +15,13 @@ * limitations under the License. */ -package org.apache.arrow.memory; +package org.apache.arrow.memory.netty; import static org.junit.Assert.assertEquals; +import org.apache.arrow.memory.ArrowBuf; +import org.apache.arrow.memory.ReferenceManager; +import org.apache.arrow.memory.RootAllocator; import org.junit.AfterClass; import org.junit.BeforeClass; import org.junit.Test; @@ -47,8 +50,8 @@ public static void afterClass() { public void testZeroBuf() { // Exercise the historical log inside the empty ArrowBuf. This is initialized statically, and there is a circular // dependency between ArrowBuf and BaseAllocator, so if the initialization happens in the wrong order, the - // historical log will be null even though BaseAllocator.DEBUG is true. - allocator.getEmpty().print(new StringBuilder(), 0, BaseAllocator.Verbosity.LOG_WITH_STACKTRACE); + // historical log will be null even though RootAllocator.DEBUG is true. + allocator.getEmpty().print(new StringBuilder(), 0, RootAllocator.Verbosity.LOG_WITH_STACKTRACE); } @Test diff --git a/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestEndianness.java b/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/netty/TestEndianness.java similarity index 92% rename from java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestEndianness.java rename to java/memory/memory-netty/src/test/java/org/apache/arrow/memory/netty/TestEndianness.java index dcaeb24889e..a782523cbc6 100644 --- a/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestEndianness.java +++ b/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/netty/TestEndianness.java @@ -15,12 +15,14 @@ * limitations under the License. */ -package org.apache.arrow.memory; +package org.apache.arrow.memory.netty; import static org.junit.Assert.assertEquals; import java.nio.ByteOrder; +import org.apache.arrow.memory.BufferAllocator; +import org.apache.arrow.memory.RootAllocator; import org.junit.Test; import io.netty.buffer.ByteBuf; diff --git a/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestNettyAllocationManager.java b/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/netty/TestNettyAllocationManager.java similarity index 87% rename from java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestNettyAllocationManager.java rename to java/memory/memory-netty/src/test/java/org/apache/arrow/memory/netty/TestNettyAllocationManager.java index 1b64cd73363..39692c96ceb 100644 --- a/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestNettyAllocationManager.java +++ b/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/netty/TestNettyAllocationManager.java @@ -15,13 +15,18 @@ * limitations under the License. */ -package org.apache.arrow.memory; +package org.apache.arrow.memory.netty; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import org.apache.arrow.memory.AllocationManager; +import org.apache.arrow.memory.ArrowBuf; +import org.apache.arrow.memory.BufferAllocator; +import org.apache.arrow.memory.BufferLedger; +import org.apache.arrow.memory.RootAllocator; import org.junit.Test; /** @@ -31,8 +36,8 @@ public class TestNettyAllocationManager { static int CUSTOMIZED_ALLOCATION_CUTOFF_VALUE = 1024; - private BaseAllocator createCustomizedAllocator() { - return new RootAllocator(BaseAllocator.configBuilder() + private RootAllocator createCustomizedAllocator() { + return new RootAllocator(RootAllocator.configBuilder() .allocationManagerFactory(new AllocationManager.Factory() { @Override public AllocationManager create(BufferAllocator accountingAllocator, long size) { @@ -65,7 +70,7 @@ private void readWriteArrowBuf(ArrowBuf buffer) { @Test public void testSmallBufferAllocation() { final long bufSize = CUSTOMIZED_ALLOCATION_CUTOFF_VALUE - 512L; - try (BaseAllocator allocator = createCustomizedAllocator(); + try (RootAllocator allocator = createCustomizedAllocator(); ArrowBuf buffer = allocator.buffer(bufSize)) { assertTrue(buffer.getReferenceManager() instanceof BufferLedger); @@ -89,7 +94,7 @@ public void testSmallBufferAllocation() { @Test public void testLargeBufferAllocation() { final long bufSize = CUSTOMIZED_ALLOCATION_CUTOFF_VALUE + 1024L; - try (BaseAllocator allocator = createCustomizedAllocator(); + try (RootAllocator allocator = createCustomizedAllocator(); ArrowBuf buffer = allocator.buffer(bufSize)) { assertTrue(buffer.getReferenceManager() instanceof BufferLedger); BufferLedger bufferLedger = (BufferLedger) buffer.getReferenceManager(); diff --git a/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/netty/TestNettyAllocator.java b/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/netty/TestNettyAllocator.java new file mode 100644 index 00000000000..b9525d0fe9d --- /dev/null +++ b/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/netty/TestNettyAllocator.java @@ -0,0 +1,75 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.arrow.memory.netty; + +import static org.junit.Assert.assertTrue; + +import java.util.stream.Collectors; + +import org.apache.arrow.memory.ArrowBuf; +import org.apache.arrow.memory.ReferenceManager; +import org.junit.Test; +import org.slf4j.LoggerFactory; + +import ch.qos.logback.classic.Level; +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.read.ListAppender; +import io.netty.buffer.PooledByteBufAllocatorL; + +/** + * Test netty allocators. + */ +public class TestNettyAllocator { + + @Test + public void testMemoryUsage() { + ListAppender memoryLogsAppender = new ListAppender<>(); + Logger logger = (Logger) LoggerFactory.getLogger("arrow.allocator"); + try { + logger.setLevel(Level.TRACE); + logger.addAppender(memoryLogsAppender); + memoryLogsAppender.start(); + try (ArrowBuf buf = new ArrowBuf(ReferenceManager.NO_OP, null, + 1024, new PooledByteBufAllocatorL().empty.memoryAddress())) { + buf.memoryAddress(); + } + boolean result = false; + long startTime = System.currentTimeMillis(); + while ((System.currentTimeMillis() - startTime) < 10000) { // 10 seconds maximum for time to read logs + result = memoryLogsAppender.list.stream() + .anyMatch( + log -> log.toString().contains("Memory Usage: \n") && + log.toString().contains("Large buffers outstanding: ") && + log.toString().contains("Normal buffers outstanding: ") && + log.getLevel().equals(Level.TRACE) + ); + if (result) { + break; + } + } + assertTrue("Log messages are:\n" + + memoryLogsAppender.list.stream().map(ILoggingEvent::toString).collect(Collectors.joining("\n")), + result); + } finally { + memoryLogsAppender.stop(); + logger.detachAppender(memoryLogsAppender); + logger.setLevel(null); + } + } +} diff --git a/java/memory/memory-unsafe/src/main/java/org/apache/arrow/memory/DefaultAllocationManagerFactory.java b/java/memory/memory-unsafe/src/main/java/org/apache/arrow/memory/unsafe/DefaultAllocationManagerFactory.java similarity index 87% rename from java/memory/memory-unsafe/src/main/java/org/apache/arrow/memory/DefaultAllocationManagerFactory.java rename to java/memory/memory-unsafe/src/main/java/org/apache/arrow/memory/unsafe/DefaultAllocationManagerFactory.java index 720c3d02d23..dfb6c706856 100644 --- a/java/memory/memory-unsafe/src/main/java/org/apache/arrow/memory/DefaultAllocationManagerFactory.java +++ b/java/memory/memory-unsafe/src/main/java/org/apache/arrow/memory/unsafe/DefaultAllocationManagerFactory.java @@ -15,7 +15,11 @@ * limitations under the License. */ -package org.apache.arrow.memory; +package org.apache.arrow.memory.unsafe; + +import org.apache.arrow.memory.AllocationManager; +import org.apache.arrow.memory.ArrowBuf; +import org.apache.arrow.memory.BufferAllocator; /** * The default Allocation Manager Factory for a module. diff --git a/java/memory/memory-unsafe/src/main/java/org/apache/arrow/memory/UnsafeAllocationManager.java b/java/memory/memory-unsafe/src/main/java/org/apache/arrow/memory/unsafe/UnsafeAllocationManager.java similarity index 89% rename from java/memory/memory-unsafe/src/main/java/org/apache/arrow/memory/UnsafeAllocationManager.java rename to java/memory/memory-unsafe/src/main/java/org/apache/arrow/memory/unsafe/UnsafeAllocationManager.java index b10aba3598d..3468a6ec65c 100644 --- a/java/memory/memory-unsafe/src/main/java/org/apache/arrow/memory/UnsafeAllocationManager.java +++ b/java/memory/memory-unsafe/src/main/java/org/apache/arrow/memory/unsafe/UnsafeAllocationManager.java @@ -15,8 +15,12 @@ * limitations under the License. */ -package org.apache.arrow.memory; +package org.apache.arrow.memory.unsafe; +import org.apache.arrow.memory.AllocationManager; +import org.apache.arrow.memory.ArrowBuf; +import org.apache.arrow.memory.BufferAllocator; +import org.apache.arrow.memory.ReferenceManager; import org.apache.arrow.memory.util.MemoryUtil; /** diff --git a/java/memory/memory-unsafe/src/test/java/org/apache/arrow/memory/TestAllocationManagerUnsafe.java b/java/memory/memory-unsafe/src/test/java/org/apache/arrow/memory/unsafe/TestAllocationManagerUnsafe.java similarity index 90% rename from java/memory/memory-unsafe/src/test/java/org/apache/arrow/memory/TestAllocationManagerUnsafe.java rename to java/memory/memory-unsafe/src/test/java/org/apache/arrow/memory/unsafe/TestAllocationManagerUnsafe.java index 33abe92e50f..f1ca96eea0f 100644 --- a/java/memory/memory-unsafe/src/test/java/org/apache/arrow/memory/TestAllocationManagerUnsafe.java +++ b/java/memory/memory-unsafe/src/test/java/org/apache/arrow/memory/unsafe/TestAllocationManagerUnsafe.java @@ -15,10 +15,12 @@ * limitations under the License. */ -package org.apache.arrow.memory; +package org.apache.arrow.memory.unsafe; import static org.junit.Assert.assertEquals; +import org.apache.arrow.memory.AllocationManager; +import org.apache.arrow.memory.DefaultAllocationManagerOption; import org.junit.Test; /** diff --git a/java/memory/memory-unsafe/src/test/java/org/apache/arrow/memory/TestUnsafeAllocationManager.java b/java/memory/memory-unsafe/src/test/java/org/apache/arrow/memory/unsafe/TestUnsafeAllocationManager.java similarity index 82% rename from java/memory/memory-unsafe/src/test/java/org/apache/arrow/memory/TestUnsafeAllocationManager.java rename to java/memory/memory-unsafe/src/test/java/org/apache/arrow/memory/unsafe/TestUnsafeAllocationManager.java index c15882a37a6..5d74c398e26 100644 --- a/java/memory/memory-unsafe/src/test/java/org/apache/arrow/memory/TestUnsafeAllocationManager.java +++ b/java/memory/memory-unsafe/src/test/java/org/apache/arrow/memory/unsafe/TestUnsafeAllocationManager.java @@ -15,11 +15,16 @@ * limitations under the License. */ -package org.apache.arrow.memory; +package org.apache.arrow.memory.unsafe; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; +import org.apache.arrow.memory.AllocationManager; +import org.apache.arrow.memory.ArrowBuf; +import org.apache.arrow.memory.BufferLedger; +import org.apache.arrow.memory.RootAllocator; +import org.apache.arrow.memory.unsafe.UnsafeAllocationManager; import org.junit.Test; /** @@ -27,8 +32,8 @@ */ public class TestUnsafeAllocationManager { - private BaseAllocator createUnsafeAllocator() { - return new RootAllocator(BaseAllocator.configBuilder().allocationManagerFactory(UnsafeAllocationManager.FACTORY) + private RootAllocator createUnsafeAllocator() { + return new RootAllocator(RootAllocator.configBuilder().allocationManagerFactory(UnsafeAllocationManager.FACTORY) .build()); } @@ -51,7 +56,7 @@ private void readWriteArrowBuf(ArrowBuf buffer) { @Test public void testBufferAllocation() { final long bufSize = 4096L; - try (BaseAllocator allocator = createUnsafeAllocator(); + try (RootAllocator allocator = createUnsafeAllocator(); ArrowBuf buffer = allocator.buffer(bufSize)) { assertTrue(buffer.getReferenceManager() instanceof BufferLedger); BufferLedger bufferLedger = (BufferLedger) buffer.getReferenceManager(); diff --git a/java/memory/pom.xml b/java/memory/pom.xml index c10263b97f1..55fbb90353f 100644 --- a/java/memory/pom.xml +++ b/java/memory/pom.xml @@ -23,6 +23,7 @@ memory-core memory-unsafe + memory-netty-buffer-patch memory-netty From 6ce984430b9e1764acb1da5c85c49294793bd590 Mon Sep 17 00:00:00 2001 From: James Duong Date: Wed, 29 Nov 2023 14:50:50 -0800 Subject: [PATCH 18/30] Add module-info for memory-core and memory-unsafe Also update arrow-vector to depend on memory-core. Netty in this PR omitted due to the complexity of needing to use patch-module on distributed JARs at run time. --- .../src/main/java/module-info.java | 28 +++++++++++++++++++ .../src/main/java/module-info.java | 20 +++++++++++++ java/vector/src/main/java/module-info.java | 1 + 3 files changed, 49 insertions(+) create mode 100644 java/memory/memory-core/src/main/java/module-info.java create mode 100644 java/memory/memory-unsafe/src/main/java/module-info.java diff --git a/java/memory/memory-core/src/main/java/module-info.java b/java/memory/memory-core/src/main/java/module-info.java new file mode 100644 index 00000000000..815f53bed63 --- /dev/null +++ b/java/memory/memory-core/src/main/java/module-info.java @@ -0,0 +1,28 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +module org.apache.arrow.memory.core { + exports org.apache.arrow.memory; + exports org.apache.arrow.memory.rounding; + exports org.apache.arrow.memory.util; + exports org.apache.arrow.memory.util.hash; + exports org.apache.arrow.util; + requires transitive jdk.unsupported; + requires jsr305; + requires org.immutables.value; + requires slf4j.api; +} diff --git a/java/memory/memory-unsafe/src/main/java/module-info.java b/java/memory/memory-unsafe/src/main/java/module-info.java new file mode 100644 index 00000000000..b8701311b36 --- /dev/null +++ b/java/memory/memory-unsafe/src/main/java/module-info.java @@ -0,0 +1,20 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +module org.apache.arrow.memory.unsafe { + requires org.apache.arrow.memory.core; +} diff --git a/java/vector/src/main/java/module-info.java b/java/vector/src/main/java/module-info.java index 346e809b854..e8a4467f51c 100644 --- a/java/vector/src/main/java/module-info.java +++ b/java/vector/src/main/java/module-info.java @@ -45,6 +45,7 @@ requires java.sql; requires jdk.unsupported; requires org.apache.arrow.format; + requires org.apache.arrow.memory.core; requires org.apache.commons.codec; requires slf4j.api; } From b83498ccab823b64b8a6336b113a246c2b1aab24 Mon Sep 17 00:00:00 2001 From: James Duong Date: Fri, 24 Nov 2023 10:05:18 -0800 Subject: [PATCH 19/30] Ignore TestBaseAllocator#testRootAllocator_getEmpty This test fails because we no longer use Netty's allocation manager factory since the test has moved into memory-core. To be fixed. --- .../src/test/java/org/apache/arrow/memory/TestBaseAllocator.java | 1 + 1 file changed, 1 insertion(+) diff --git a/java/memory/memory-core/src/test/java/org/apache/arrow/memory/TestBaseAllocator.java b/java/memory/memory-core/src/test/java/org/apache/arrow/memory/TestBaseAllocator.java index 365c84f5a2b..f90e2214215 100644 --- a/java/memory/memory-core/src/test/java/org/apache/arrow/memory/TestBaseAllocator.java +++ b/java/memory/memory-core/src/test/java/org/apache/arrow/memory/TestBaseAllocator.java @@ -112,6 +112,7 @@ public void testRootAllocator_closeWithOutstanding() throws Exception { } @Test + @Ignore public void testRootAllocator_getEmpty() throws Exception { try (final RootAllocator rootAllocator = new RootAllocator(MAX_ALLOCATION)) { From b682dda7084b0c8670d0efb34a1dea07b1cb196e Mon Sep 17 00:00:00 2001 From: James Duong Date: Tue, 28 Nov 2023 14:52:46 -0800 Subject: [PATCH 20/30] Edit TestArrowBuf#testEnabledHistoricalLog to have more info Log what message was reported if the message didn't contain the correct information. --- .../test/java/org/apache/arrow/memory/TestArrowBuf.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/java/memory/memory-core/src/test/java/org/apache/arrow/memory/TestArrowBuf.java b/java/memory/memory-core/src/test/java/org/apache/arrow/memory/TestArrowBuf.java index c88768c43bc..9ba42abc1ce 100644 --- a/java/memory/memory-core/src/test/java/org/apache/arrow/memory/TestArrowBuf.java +++ b/java/memory/memory-core/src/test/java/org/apache/arrow/memory/TestArrowBuf.java @@ -167,13 +167,15 @@ public void testEnabledHistoricalLog() { fieldDebug.set(null, true); try (BufferAllocator allocator = new RootAllocator(128)) { allocator.buffer(2); - Exception e = assertThrows(IllegalStateException.class, () -> allocator.close()); - assertTrue(e.getMessage().contains("event log for:")); // JDK8, JDK11 + Exception e = assertThrows(IllegalStateException.class, allocator::close); + assertTrue("Exception had the following message: " + e.getMessage(), + e.getMessage().contains("event log for:")); // JDK8, JDK11 } finally { fieldDebug.set(null, false); } } catch (Exception e) { - assertTrue(e.toString().contains("java.lang.NoSuchFieldException: modifiers")); // JDK17+ + assertTrue("Exception had the following toString(): " + e.toString(), + e.toString().contains("java.lang.NoSuchFieldException: modifiers")); // JDK17+ } finally { ((Logger) LoggerFactory.getLogger("org.apache.arrow")).setLevel(null); } From a7948124eaaf2732ee3d7f68da60ff2a10e99f84 Mon Sep 17 00:00:00 2001 From: James Duong Date: Wed, 29 Nov 2023 15:02:58 -0800 Subject: [PATCH 21/30] Allow use of reflection on Unsafe from memory-core --- java/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/pom.xml b/java/pom.xml index 7dd75528fe6..e03efbd5a50 100644 --- a/java/pom.xml +++ b/java/pom.xml @@ -889,7 +889,7 @@ org.apache.maven.plugins maven-surefire-plugin - --add-opens=java.base/java.nio=ALL-UNNAMED + --add-opens=java.base/java.nio=org.apache.arrow.memory.core,ALL-UNNAMED From a36130c45cb81dabf498ed82941757b1e4bed8df Mon Sep 17 00:00:00 2001 From: James Duong Date: Wed, 29 Nov 2023 15:23:09 -0800 Subject: [PATCH 22/30] Provide better error message about needing memory-core to have access to unsafe --- .../src/main/java/org/apache/arrow/memory/util/MemoryUtil.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/MemoryUtil.java b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/MemoryUtil.java index cc615c5b383..12d2ae4ce66 100644 --- a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/MemoryUtil.java +++ b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/MemoryUtil.java @@ -142,7 +142,7 @@ public Object run() { // the static fields above get initialized final RuntimeException failure = new RuntimeException( "Failed to initialize MemoryUtil. You must start Java with " + - "`--add-opens=java.base/java.nio=ALL-UNNAMED` " + + "`--add-opens=java.base/java.nio=org.apache.arrow.memory.core,ALL-UNNAMED` " + "(See https://arrow.apache.org/docs/java/install.html)", e); failure.printStackTrace(); throw failure; From 57dd15130707a98c48882be28f5d51e22c8f05a1 Mon Sep 17 00:00:00 2001 From: James Duong Date: Wed, 29 Nov 2023 15:30:08 -0800 Subject: [PATCH 23/30] Allow memory-core to use reflection on memory-unsafe --- java/memory/memory-unsafe/src/main/java/module-info.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/java/memory/memory-unsafe/src/main/java/module-info.java b/java/memory/memory-unsafe/src/main/java/module-info.java index b8701311b36..aa340d21716 100644 --- a/java/memory/memory-unsafe/src/main/java/module-info.java +++ b/java/memory/memory-unsafe/src/main/java/module-info.java @@ -16,5 +16,7 @@ */ module org.apache.arrow.memory.unsafe { + exports org.apache.arrow.memory.unsafe to org.apache.arrow.memory.core; + requires org.apache.arrow.memory.core; } From e3e707769cb2249137732d76d8071b5c891b4731 Mon Sep 17 00:00:00 2001 From: James Duong Date: Thu, 30 Nov 2023 14:39:41 -0800 Subject: [PATCH 24/30] Add explicit dependency on immutables to memory-unsafe --- java/memory/memory-unsafe/pom.xml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/java/memory/memory-unsafe/pom.xml b/java/memory/memory-unsafe/pom.xml index 9f813730819..5ef4e8a9149 100644 --- a/java/memory/memory-unsafe/pom.xml +++ b/java/memory/memory-unsafe/pom.xml @@ -27,6 +27,10 @@ org.apache.arrow arrow-memory-core + + org.immutables + value + From 478446a73f3b07d4d801ecd7b55798684b586c9b Mon Sep 17 00:00:00 2001 From: James Duong Date: Thu, 30 Nov 2023 15:06:57 -0800 Subject: [PATCH 25/30] Enable reflection during arrow-memory-core tests The tests themselves (specifically testEnableHistoricalLog) use reflection on java.lang.reflect so surefire needs to enable this permission. --- java/memory/memory-core/pom.xml | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/java/memory/memory-core/pom.xml b/java/memory/memory-core/pom.xml index 8f286990453..66be9c2ff4e 100644 --- a/java/memory/memory-core/pom.xml +++ b/java/memory/memory-core/pom.xml @@ -42,7 +42,6 @@ org.apache.maven.plugins maven-surefire-plugin - @@ -54,6 +53,30 @@ + + error-prone-jdk11+ + + [11,] + + !m2e.version + + + + + + org.apache.maven.plugins + maven-surefire-plugin + + --add-opens=java.base/java.lang.reflect=org.apache.arrow.memory.core --add-opens=java.base/java.nio=org.apache.arrow.memory.core,ALL-UNNAMED + + + **/TestOpens.java + + + + + + opens-tests From c792b63cd5b1617afa0cda30c8f53dae1e3117bd Mon Sep 17 00:00:00 2001 From: James Duong Date: Thu, 30 Nov 2023 15:56:00 -0800 Subject: [PATCH 26/30] Fix occasionally ConcurrentModificationException in Netty tests Due to using stream() on a log appender while the appender is still running and possibly adding more logs. --- .../memory/netty/TestNettyAllocator.java | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/netty/TestNettyAllocator.java b/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/netty/TestNettyAllocator.java index b9525d0fe9d..07fdc3f784e 100644 --- a/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/netty/TestNettyAllocator.java +++ b/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/netty/TestNettyAllocator.java @@ -19,6 +19,7 @@ import static org.junit.Assert.assertTrue; +import java.util.Collections; import java.util.stream.Collectors; import org.apache.arrow.memory.ArrowBuf; @@ -40,6 +41,7 @@ public class TestNettyAllocator { @Test public void testMemoryUsage() { ListAppender memoryLogsAppender = new ListAppender<>(); + memoryLogsAppender.list = Collections.synchronizedList(memoryLogsAppender.list); Logger logger = (Logger) LoggerFactory.getLogger("arrow.allocator"); try { logger.setLevel(Level.TRACE); @@ -52,13 +54,17 @@ public void testMemoryUsage() { boolean result = false; long startTime = System.currentTimeMillis(); while ((System.currentTimeMillis() - startTime) < 10000) { // 10 seconds maximum for time to read logs - result = memoryLogsAppender.list.stream() - .anyMatch( - log -> log.toString().contains("Memory Usage: \n") && - log.toString().contains("Large buffers outstanding: ") && - log.toString().contains("Normal buffers outstanding: ") && - log.getLevel().equals(Level.TRACE) - ); + // Lock on the list backing the appender since a background thread might try to add more logs + // while stream() is iterating over list elements. This would throw a flakey ConcurrentModificationException. + synchronized (memoryLogsAppender.list) { + result = memoryLogsAppender.list.stream() + .anyMatch( + log -> log.toString().contains("Memory Usage: \n") && + log.toString().contains("Large buffers outstanding: ") && + log.toString().contains("Normal buffers outstanding: ") && + log.getLevel().equals(Level.TRACE) + ); + } if (result) { break; } From 4e4a6bac383239e2cc993374ed997252380aaa11 Mon Sep 17 00:00:00 2001 From: James Duong Date: Wed, 29 Nov 2023 14:51:03 -0800 Subject: [PATCH 27/30] Add netty module-info.java --- .../src/main/java/module-info.java | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 java/memory/memory-netty/src/main/java/module-info.java diff --git a/java/memory/memory-netty/src/main/java/module-info.java b/java/memory/memory-netty/src/main/java/module-info.java new file mode 100644 index 00000000000..33649b6a9f3 --- /dev/null +++ b/java/memory/memory-netty/src/main/java/module-info.java @@ -0,0 +1,22 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +module org.apache.arrow.memory.netty { + requires org.apache.arrow.memory.core; + requires io.netty.buffer; + requires io.netty.common; +} From 0d44ba1c522c16ec1dbeeba090f7c7f37c0e7f65 Mon Sep 17 00:00:00 2001 From: James Duong Date: Tue, 28 Nov 2023 03:36:23 -0800 Subject: [PATCH 28/30] Update the surefire command line to for Netty modules Need to patch Netty at runtime and enable reflection --- java/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/pom.xml b/java/pom.xml index e03efbd5a50..fc9ed04f27b 100644 --- a/java/pom.xml +++ b/java/pom.xml @@ -889,7 +889,7 @@ org.apache.maven.plugins maven-surefire-plugin - --add-opens=java.base/java.nio=org.apache.arrow.memory.core,ALL-UNNAMED + --add-opens java.base/jdk.internal.misc=io.netty.common --patch-module=io.netty.buffer=${project.basedir}/../memory-netty-buffer-patch/target/arrow-memory-netty-buffer-patch-${project.version}.jar --add-opens=java.base/java.nio=org.apache.arrow.memory.core,io.netty.common,ALL-UNNAMED From 0d9d03dcdce2f2ee1dd6e182a6fa594e0ba9b3b1 Mon Sep 17 00:00:00 2001 From: James Duong Date: Wed, 29 Nov 2023 15:31:02 -0800 Subject: [PATCH 29/30] Allow memory-core to use reflection on memory-netty --- java/memory/memory-netty/src/main/java/module-info.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/java/memory/memory-netty/src/main/java/module-info.java b/java/memory/memory-netty/src/main/java/module-info.java index 33649b6a9f3..95d1cdbd11f 100644 --- a/java/memory/memory-netty/src/main/java/module-info.java +++ b/java/memory/memory-netty/src/main/java/module-info.java @@ -16,6 +16,8 @@ */ module org.apache.arrow.memory.netty { + exports org.apache.arrow.memory.netty to org.apache.arrow.memory.core; + requires org.apache.arrow.memory.core; requires io.netty.buffer; requires io.netty.common; From 7a9ba6f40bdd75afa3cf66ff3e815b8177c70329 Mon Sep 17 00:00:00 2001 From: James Duong Date: Tue, 28 Nov 2023 03:41:29 -0800 Subject: [PATCH 30/30] Restructure Netty tests for modules Move tests outside of io.netty.buffer package because that causes a conflict with Netty's modules. Change functions marked as protected that are used in tests to be public to facilitate this refactoring. --- .../java/io/netty/buffer/NettyArrowBuf.java | 17 +++++++++++------ .../io/netty/buffer/TestExpandableByteBuf.java | 2 +- .../io/netty/buffer/TestNettyArrowBuf.java | 2 +- 3 files changed, 13 insertions(+), 8 deletions(-) rename java/memory/memory-netty/src/test/java/{ => org/apache/arrow}/io/netty/buffer/TestExpandableByteBuf.java (99%) rename java/memory/memory-netty/src/test/java/{ => org/apache/arrow}/io/netty/buffer/TestNettyArrowBuf.java (99%) diff --git a/java/memory/memory-netty-buffer-patch/src/main/java/io/netty/buffer/NettyArrowBuf.java b/java/memory/memory-netty-buffer-patch/src/main/java/io/netty/buffer/NettyArrowBuf.java index 466444c7d53..a5a5c2d40bc 100644 --- a/java/memory/memory-netty-buffer-patch/src/main/java/io/netty/buffer/NettyArrowBuf.java +++ b/java/memory/memory-netty-buffer-patch/src/main/java/io/netty/buffer/NettyArrowBuf.java @@ -436,7 +436,8 @@ public byte getByte(int index) { } @Override - protected short _getShortLE(int index) { + @VisibleForTesting + public short _getShortLE(int index) { short s = getShort(index); return Short.reverseBytes(s); } @@ -452,7 +453,8 @@ public short getShort(int index) { } @Override - protected int _getIntLE(int index) { + @VisibleForTesting + public int _getIntLE(int index) { int value = getInt(index); return Integer.reverseBytes(value); } @@ -468,7 +470,8 @@ public int getInt(int index) { } @Override - protected long _getLongLE(int index) { + @VisibleForTesting + public long _getLongLE(int index) { long value = getLong(index); return Long.reverseBytes(value); } @@ -509,7 +512,8 @@ protected void _setShortLE(int index, int value) { } @Override - protected void _setShort(int index, int value) { + @VisibleForTesting + public void _setShort(int index, int value) { setShort(index, value); } @@ -581,7 +585,7 @@ public NettyArrowBuf setMedium(int index, int value) { @Override @VisibleForTesting - protected void _setInt(int index, int value) { + public void _setInt(int index, int value) { setInt(index, value); } @@ -599,7 +603,8 @@ public NettyArrowBuf setInt(int index, int value) { } @Override - protected void _setLong(int index, long value) { + @VisibleForTesting + public void _setLong(int index, long value) { setLong(index, value); } diff --git a/java/memory/memory-netty/src/test/java/io/netty/buffer/TestExpandableByteBuf.java b/java/memory/memory-netty/src/test/java/org/apache/arrow/io/netty/buffer/TestExpandableByteBuf.java similarity index 99% rename from java/memory/memory-netty/src/test/java/io/netty/buffer/TestExpandableByteBuf.java rename to java/memory/memory-netty/src/test/java/org/apache/arrow/io/netty/buffer/TestExpandableByteBuf.java index 67557b65a62..b4a7a70de73 100644 --- a/java/memory/memory-netty/src/test/java/io/netty/buffer/TestExpandableByteBuf.java +++ b/java/memory/memory-netty/src/test/java/org/apache/arrow/io/netty/buffer/TestExpandableByteBuf.java @@ -15,7 +15,7 @@ * limitations under the License. */ -package io.netty.buffer; +package org.apache.arrow.io.netty.buffer; import org.apache.arrow.memory.ArrowBuf; import org.apache.arrow.memory.BufferAllocator; diff --git a/java/memory/memory-netty/src/test/java/io/netty/buffer/TestNettyArrowBuf.java b/java/memory/memory-netty/src/test/java/org/apache/arrow/io/netty/buffer/TestNettyArrowBuf.java similarity index 99% rename from java/memory/memory-netty/src/test/java/io/netty/buffer/TestNettyArrowBuf.java rename to java/memory/memory-netty/src/test/java/org/apache/arrow/io/netty/buffer/TestNettyArrowBuf.java index f18bccb4c9d..dc715d9b1ac 100644 --- a/java/memory/memory-netty/src/test/java/io/netty/buffer/TestNettyArrowBuf.java +++ b/java/memory/memory-netty/src/test/java/org/apache/arrow/io/netty/buffer/TestNettyArrowBuf.java @@ -15,7 +15,7 @@ * limitations under the License. */ -package io.netty.buffer; +package org.apache.arrow.io.netty.buffer; import java.nio.ByteBuffer;