OCamlPro / gnucobol

A clone of the sourceforge GnuCOBOL compiler from COBOL to C.
https://get-superbol.com
GNU Lesser General Public License v3.0
22 stars 24 forks source link

Further CI adjustments for Java interoperability branch #169

Closed nberth closed 3 months ago

nberth commented 3 months ago

@GitMensch At the moment the tests with a call to Java fail:

1264. run_java.at:23: testing CALL Java static void (void) ...
../../tests/run_java.at:43: $JAVAC Test.java
../../tests/run_java.at:44: $COMPILE prog.cob
../../tests/run_java.at:45: $COBCRUN_DIRECT ./prog
--- /dev/null   2024-08-16 07:15:45.000000000 +0000
+++ /d/a/gnucobol/gnucobol/_build/tests/testsuite.dir/at-groups/1264/stderr 2024-08-16 07:15:45.467925900 +0000
@@ -0,0 +1 @@
+libcob: prog.cob:5: error: Java interoperability module cannot be loaded: module libcobjni-1.dll not found
--- -   2024-08-16 07:15:45.576799400 +0000
+++ /d/a/gnucobol/gnucobol/_build/tests/testsuite.dir/at-groups/1264/stdout 2024-08-16 07:15:45.530426900 +0000
@@ -1,2 +1 @@
-Hello world!

1264. run_java.at:23: 1264. CALL Java static void (void) (run_java.at:23): FAILED (run_java.at:45)

#                             -*- compilation -*-
1265. run_java.at:51: testing CALL Java static void (void) (missing) ...
../../tests/run_java.at:64: $COMPILE prog.cob
../../tests/run_java.at:66: $COBCRUN_DIRECT ./prog
--- /dev/null   2024-08-16 07:15:45.000000000 +0000
+++ /d/a/gnucobol/gnucobol/_build/tests/testsuite.dir/at-groups/1265/stderr 2024-08-16 07:15:45.123619000 +0000
@@ -0,0 +1 @@
+libcob: prog.cob:5: error: Java interoperability module cannot be loaded: module libcobjni-1.dll not found
1265. run_java.at:51: 1265. CALL Java static void (void) (missing) (run_java.at:51): FAILED (run_java.at:66)

In install.log we have:

…
/opt/cobol/gnucobol/bin
/opt/cobol/gnucobol/bin/cob-config
/opt/cobol/gnucobol/bin/cobc.exe
/opt/cobol/gnucobol/bin/cobcrun.exe
/opt/cobol/gnucobol/bin/cobfile.exe
/opt/cobol/gnucobol/bin/gcdiff.exe
/opt/cobol/gnucobol/bin/libcob-5.dll
/opt/cobol/gnucobol/bin/libcobdb-1.dll
/opt/cobol/gnucobol/bin/libcobjni-1.dll
…

so that lib seems to be built. I'm not sure what part is missing for the dynamic lookup to operate properly.

GitMensch commented 3 months ago

I'm not sure what part is missing for the dynamic lookup to operate properly.

It would be good if someone merges https://github.com/OCamlPro/gnucobol/commit/12313877989cde508eb443f911927014566f3574 and by doing so applying these changes also to cob_load_lib() (ideally in svn upstream, of course) - this should provide us with more details for this (and other) failure(s).

nberth commented 3 months ago

Thanks @GitMensch for your review.

jvm.dll is always in $JAVA_HOME/bin/server, isn't it?

Is it a question or an assertion? I don't know the answer, as I said I'm completely unfamiliar with how things are done on Windows.

Wouldn't it be reasonable then to check if JAVA_HOME is set, and if yes use it concatenated with "bin/server", passing it to lt_dladdsearchdir()? If we then cannot delay-load libcob-java,m we can also hint the user to set JAVA_HOME.

If $JAVA_HOME/bin/server is always where we can find jvm.dll, then yes I guess we can rely more on that variable if it is set.

GitMensch commented 3 months ago

Thanks @GitMensch for your review.

You're welcome. Thanks for taking care in the first place!

jvm.dll is always in $JAVA_HOME/bin/server, isn't it?

Is it a question or an assertion? I don't know the answer, as I said I'm completely unfamiliar with how things are done on Windows.

That seems more JVM related than Windows - it is what I've found in multiple Windows installs using JRE since 8 (aka 1.8.0) up top current ones with both x64 and x86 architectures... While I just have seen 1 very old installation that has it under $JAVA_HOME/bin/client... checking further led to https://www.oracle.com/java/technologies/ms-windows-install-64bit.html#jvm.dll, so these two seem to be the official paths.

While this can be all made up, the following answer of ChatGPT4 seems quite reasonable and, from a glance, matches what https://www.oracle.com/java/technologies/whitepaper.html#overview documents:

The jvm.dll files located in JAVA_HOME/bin/server and JAVA_HOME/bin/client represent different implementations of the Java Virtual Machine (JVM) provided by the Java Development Kit (JDK). Here are the key differences:

Client JVM (JAVA_HOME/bin/client/jvm.dll)

  1. Optimization for Client-Side Applications: The client JVM is optimized for quick startup and responsive performance suitable for client-side applications, such as desktop applications.
  2. Memory Usage: It typically uses less memory than the server JVM, making it more appropriate for environments where resources are limited.
  3. Compilation Strategy: The client JVM uses a simpler, less aggressive compilation strategy. It focuses on reducing the time it takes to start up the application rather than on long-term performance optimization.
  4. Garbage Collection: The client JVM uses a less aggressive garbage collection strategy, aiming to minimize the perceived pauses for applications.

Server JVM (JAVA_HOME/bin/server/jvm.dll)

  1. Optimization for Server-Side Applications: The server JVM is designed for long-running server applications, such as web servers or application servers, where performance and throughput are more critical than startup time.
  2. Memory Usage: It may use more memory as it aims to optimize for long-term performance rather than conserving resources.
  3. Compilation Strategy: The server JVM employs a more aggressive compilation strategy, including advanced optimizations such as Just-In-Time (JIT) compilation. This results in better performance over time as the JVM can make more sophisticated optimizations based on the application's behavior.
  4. Garbage Collection: The server JVM uses more sophisticated and aggressive garbage collection algorithms, which are designed to improve throughput and reduce long-term memory fragmentation.

Usage

  • Client JVM: Best for desktop applications where fast startup and responsiveness are important.
  • Server JVM: Ideal for server applications where long-term performance and throughput are critical.

How to Choose

The choice between the client and server JVM typically depends on the nature of the application you are running:

  • For short-lived, interactive applications, you might prefer the client JVM.
  • For long-running, resource-intensive applications, the server JVM would be the better choice.

Selection at Runtime

When you run a Java application, you can specify which JVM to use. If no specific JVM is mentioned, the JDK typically defaults to one based on the operating system and architecture. You can explicitly select the server or client JVM by specifying the -client or -server options when starting your Java application:

java -client -jar MyApp.jar

or

java -server -jar MyApp.jar

These options ensure that the appropriate jvm.dll is used, corresponding to the chosen JVM implementation.

which brings us to...

Wouldn't it be reasonable then to check if JAVA_HOME is set, and if yes use it concatenated with "bin/server", passing it to lt_dladdsearchdir()? If we then cannot delay-load libcob-java,m we can also hint the user to set JAVA_HOME.

If $JAVA_HOME/bin/server is always where we can find jvm.dll, then yes I guess we can rely more on that variable if it is set.

I'd say for now it would be reasonable to append both the server and the client directory (in this order) to the search path, as soon as JAVA_HOME is set. Please adjust it that way (doing that in the code for WIN32 and dropping it from atlocal).

For Linux this is the libvjm.so. Checking some Linux distros shows that (on older OS?) there's a subfolder in the system package manager installs containing the architecture but binary jdk downloads (and newer OS?) have those "directly" under $JAVA_HOME/lib/server/libjvm.so (or $JAVA_HOME/lib/client/libjvm.so).

Maybe that's overkill, but what do you think of the following:

GitMensch commented 3 months ago

... second thought about "JVM_LIB_PATH": as people can self-prefix the system specific search dir we likely can just drop that extra variable and just append to the search path according to JAVA_HOME

nberth commented 3 months ago

Agreed ;-)

nberth commented 3 months ago

@GitMensch While investigating this I'm wondering whether one should always assume libltdl is used (as it provides lt_dladdsearchdir); other variants for similar systems do not appear do offer any search path facility. On WIN32 there a AddDllDirectory that seems available, but that [may affect DLL search order] (https://learn.microsoft.com/en-us/windows/win32/api/Winbase/nf-winbase-setdlldirectorya).

nberth commented 3 months ago

BTW configuring with --without-dl leads to a failing test:

#                             -*- compilation -*-
737. run_misc.at:4198: testing COB_PRE_LOAD with entry points ...
/home/nico/work/repos/gnucobol/tests/run_misc.at:4257: $COMPILE_MODULE prog.cob
/home/nico/work/repos/gnucobol/tests/run_misc.at:4258: $COMPILE_MODULE prog1.cob
/home/nico/work/repos/gnucobol/tests/run_misc.at:4259: $COMPILE main-prog.cob
/home/nico/work/repos/gnucobol/tests/run_misc.at:4260: COB_PRE_LOAD="prog"$PATHSEP"prog1" $COBCRUN_DIRECT ./main-prog
--- -   2024-08-19 16:36:39.341893156 +0200
+++ /tmp/b/tests/testsuite.dir/at-groups/737/stdout 2024-08-19 16:36:39.334951092 +0200
@@ -1,4 +1,4 @@
 12abc
-11
+55
 xxxxx

737. run_misc.at:4198: 737. COB_PRE_LOAD with entry points (run_misc.at:4198): FAILED (run_misc.at:4260)

Looking more and more like a rabbit hole.

GitMensch commented 3 months ago

Hm... that change gets bigger than expected...

Suggestion:

nberth commented 3 months ago

the solution looks nice to me (ChangeLogs entries missing including the separate one for the reasonable move to the systfines.h - if they are still needed in fileio.c, likely they can be kept in call.c only?)

I moved the non-CI related changes back into PR #157, so discussion on the approach for loading JNI may resume there. I'll do the fixes to the ChangeLog. In this PR there should only be one commit for CI adjustments.

Concerning the failing MacOS CI - is JAVA_HOME set there?

Yes it is set already; but the issue is fixed anyways .

nberth commented 3 months ago

https://github.com/OCamlPro/gnucobol/actions/runs/10486974200/job/29046364090?pr=169#step:8:230

I'll disable mingw32 as I'm unsable to find what package to install to get that on i686 arch. Unless @ddeclerck @GitMensch have an idea about it?

GitMensch commented 3 months ago

Better drop BDB, cjson and xml2 from all java CI definitions and explicit request --without-indexed --without-json --without-xml2 - those are (to be) tested in the "common" CI ones.

I personally would not use pacboy for the matrix but the "good old" standard one:

  strategy:
    matrix:
      include:
        - { sys: mingw64, env: x86_64 }
        - { sys: mingw32, env: i686 }
        - { sys: ucrt64,  env: ucrt-x86_64 }
        - { sys: clang64, env: clang-x86_64 }
  steps:
    - uses: msys2/setup-msys2@v2
      with:
        msystem: ${{matrix.sys}}
        install: mingw-w64-${{matrix.env}}-openssl

You can then also use the default dependencies from msys2 https://github.com/msys2/MINGW-packages/blob/67aee9a6de4f153729397c6a5b09fd8bd39786ca/mingw-w64-gnucobol/PKGBUILD#L22-L28 - but as noted, those dependencies should get into the "common" CI for msys2, along with adding that build matrix to #170.

GitMensch commented 3 months ago

As the failing Ubuntu one is not related to java-interop you may still merge that. But please keep the distcheck (only on Ubuntu) as this validates we can distribute the package.

You want to use:

make -C _build distcheck DISTCHECK_CONFIGURE_FLAGS="--without-indexed --without-json --without-xml2 --with-java" TESTSUITEFLAGS="--jobs=$((${NPROC}+1))"

nberth commented 3 months ago

As the failing Ubuntu one is not related to java-interop you may still merge that. But please keep the distcheck (only on Ubuntu) as this validates we can distribute the package.

You want to use:

make -C _build distcheck DISTCHECK_CONFIGURE_FLAGS="--without-indexed --without-json --without-xml2 --with-java" TESTSUITEFLAGS="--jobs=$((${NPROC}+1))"

Many thanks for this suggestion! I suspected there would be one variable to pass configure args, but I though since it's only for Java interop. we could discard the distribution. I'll do the change ASAP.