luhenry commented on a change in pull request #32253:
URL: https://github.com/apache/spark/pull/32253#discussion_r619069785
##########
File path: mllib-local/pom.xml
##########
@@ -75,48 +75,12 @@
<type>test-jar</type>
<scope>test</scope>
</dependency>
+
+ <dependency>
+ <groupId>dev.ludovic.netlib</groupId>
+ <artifactId>blas</artifactId>
+ </dependency>
</dependencies>
- <profiles>
- <profile>
- <id>netlib-lgpl</id>
Review comment:
> First some background - I think the issue with licensing comes from
the submodules that are pulled in by all, like:
https://github.com/fommil/netlib-java/blob/master/native_ref/xbuilds/linux-x86_64/pom.xml
netlib-java generates and compiles native code. So things like
https://mvnrepository.com/artifact/com.github.fommil.netlib/netlib-native_system-linux-x86_64/1.1
ship a .so and I believe it's that code or what it links to that is LGPL. I've
honestly forgotten the details but seemed pretty clear (it may be, at least,
libgfortran). At least, I recall going over this with a fine-toothed comb with
Cloudera's legal department and they agreed, FWIW.
I can confirm that both `netlib_ref` and `netlib_system` do link to
`libgfortran` and which is licensed with LGPL. See
[[1]](https://github.com/fommil/netlib-java/blob/master/native_ref/xbuilds/linux-x86_64/pom.xml#L66)
and
[[2]](https://github.com/fommil/netlib-java/blob/master/native_system/xbuilds/linux-x86_64/pom.xml#L77).
> Anyway, that's the reason behind all this bother with profiles. The
wrapper in netlib-java is OK to include in distributions and compile against.
But adding in the dependencies that bring in native code is not. Hence it's
under a profile that downstream users can turn on for their own builds if
desired.
So if I understand correctly, the limitation is that any code that
statically or dynamically links to any LGPL library is out of the picture for
_distribution_ with Spark. And if any downstream user wants to do it, they can
but the Open Source Spark project won't. Is that correct?
> So, my question is, where is the native code coming in here to do the
acceleration?
The main difference that `dev.ludovic.netlib` brings compared to
`com.github.fommil.netlib:native_ref` or even
`com.github.fommil.netlib:native_system` is that it doesn't link explicitely to
any native library (OpenBLAS, MKL, libgfortran) at build-time (there is no
`-l...` linker flag). Instead, it probes at run-time (via `dlopen`/`dlsym`) for
a library which name is `libblas.so` (or anything else specified by the user)
and which exposes certain symbols (`cblas_dgemm`, `cblas_ddot`, for example).
It doesn't distribute any such library and it doesn't link to any such library
(statically or dynamically) or any of their dependencies.
If the user wants to use OpenBLAS or Intel MKL, or any other BLAS-like
library for that matter, it's up to them to install it on their system. Then,
either the library is named `libblas.so` (or there is such a symbolic
filesystem link) and is accessible via LD_LIBRARY_PATH, or the user specifies
the name with `-Ddev.ludovic.netlib.blas.nativeLib` or full path with
`-Ddev.ludovic.netlib.blas.nativeLibPath`.
Overall `dev.ludovic.netlib` doesn't link to, distribute, nor even favor any
specific native BLAS implementation. So, by extension, Spark doesn't link to,
distribute, nor favor any specific native BLAS implementation or external
library (like libgfortran) with this patch.
> I understand Java 16+ has it built in, that's the whole point of your
change, but we still want to let people flip on dependencies that will build in
bindings to OpenBLAS, etc. Where would that be now?
`dev.ludovic.netlib.blas.ForeignBLAS` does load an arbitrary `libblas.so`
library and probes for symbols like `cblas_dgemm`, `cblas_ddot`, etc. It's
using `dlopen`/`dlsym` in the JVM directly.
> because I see you're just depending on netlib-java core, which doesn't
have that. I'm asking if that's what blas is, etc, but not sure that's it
either.
`blas` contains two things:
1. the java implementations of the BLAS libraries with fallbacks to F2jBLAS
(that's VectorizedBLAS for example)
2. the bindings for native BLAS libraries (with the Foreign Linker API for
now, and JNI-based bindings in the future).
The contention point is in `2. the binding for native BLAS libraries` AFAIU.
If it's a blocker, I should be able to break it down into `blas` and
`blas-native` packages for example.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
