Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ZstdCompression based on Apache Commons Compress #111

Closed

Conversation

mkitti
Copy link
Contributor

@mkitti mkitti commented Dec 3, 2023

Note tests will fail since the optional dependency zstd-jni
is not a dependency.

  1. For this to work, I recommend at least adding zstd-jni as a test dependency. Without this patch, a4a5ac9 will fail tests. With this patch applied as in e7abc25 the tests will now succeed.
diff --git a/pom.xml b/pom.xml
index 17ededb..abde2d1 100644
--- a/pom.xml
+++ b/pom.xml
@@ -208,6 +208,12 @@
                        <groupId>org.apache.commons</groupId>
                        <artifactId>commons-compress</artifactId>
                </dependency>
+               <dependency>
+                   <groupId>com.github.luben</groupId>
+                   <artifactId>zstd-jni</artifactId>
+                   <version>1.5.5-10</version>
+                   <scope>test</scope>
+               </dependency>
        </dependencies>
 
        <repositories>
  1. We should also consider adding zstd-jni as an optional dependency of n5.
  2. Another important parameter to consider is nbWorkers which controls the number of threads that Zstandard will use.

Note tests will fail since the optional dependency zstd-jni
is not a dependency.
@mkitti
Copy link
Contributor Author

mkitti commented Dec 3, 2023

From email sent on December 2nd 8:25 pm:

Note that zstd-jni is an optional dependency of Apache Commons Compress.

https://github.com/apache/commons-compress/blob/master/pom.xml#L107

https://maven.apache.org/guides/introduction/introduction-to-optional-and-excludes-dependencies.html

@mkitti
Copy link
Contributor Author

mkitti commented Dec 3, 2023

a4a5ac9 without any pom.xml changes fails with the following errors.

Error:  Tests run: 33, Failures: 0, Errors: 9, Skipped: 0, Time elapsed: 7.716 s <<< FAILURE! -- in org.janelia.saalfeldlab.n5.N5FSTest
Error:  org.janelia.saalfeldlab.n5.N5FSTest.testWriteReadIntBlock -- Time elapsed: 1.649 s <<< ERROR!
java.lang.NoClassDefFoundError: com/github/luben/zstd/ZstdOutputStream
	at org.apache.commons.compress.compressors.zstandard.ZstdCompressorOutputStream.<init>(ZstdCompressorOutputStream.java:57)
	at org.janelia.saalfeldlab.n5.ZstdCompression.getOutputStream(ZstdCompression.java:80)
	at org.janelia.saalfeldlab.n5.DefaultBlockWriter.write(DefaultBlockWriter.java:49)
	at org.janelia.saalfeldlab.n5.DefaultBlockWriter.writeBlock(DefaultBlockWriter.java:96)
	at org.janelia.saalfeldlab.n5.GsonKeyValueN5Writer.writeBlock(GsonKeyValueN5Writer.java:221)
	at org.janelia.saalfeldlab.n5.AbstractN5Test.testWriteReadIntBlock(AbstractN5Test.java:280)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:63)
	at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329)
	at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:413)
	at org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:316)
	at org.apache.maven.surefire.junit4.JUnit4Provider.executeWithRerun(JUnit4Provider.java:240)
	at org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:214)
	at org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:155)
	at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:385)
	at org.apache.maven.surefire.booter.ForkedBooter.execute(ForkedBooter.java:162)
	at org.apache.maven.surefire.booter.ForkedBooter.run(ForkedBooter.java:507)
	at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:495)
Caused by: java.lang.ClassNotFoundException: com.github.luben.zstd.ZstdOutputStream
	at java.net.URLClassLoader.findClass(URLClassLoader.java:387)
	at java.lang.ClassLoader.loadClass(ClassLoader.java:419)
	at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:352)
	at java.lang.ClassLoader.loadClass(ClassLoader.java:352)
	... 35 more

@mkitti
Copy link
Contributor Author

mkitti commented Dec 4, 2023

An alternative implementation would use the pure Java implementation of Zstandard: https://github.com/airlift/aircompressor/blob/master/src/main/java/io/airlift/compress/zstd/ZstdDecompressor.java

@mkitti
Copy link
Contributor Author

mkitti commented Dec 5, 2023

I'm closing this in favor of return to https://github.com/JaneliaSciComp/n5-zstandard

@mkitti mkitti closed this Dec 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant