bugGNU Octave - Bugs: bug #55174, java warning about ...

 
 

bug #55174: java warning about java.system.class.loader property

Submitter:  CarnĂ« Draug <carandraug>
Submitted:  Thu 06 Dec 2018 07:36:50 PM UTC
   
 
Category:  Interpreter Severity:  3 - Normal
Priority:  5 - Normal Item Group:  None
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * dev Operating System:  * GNU/Linux
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Sat 20 Jun 2020 04:30:58 PM UTC, comment #21: 

@Philip: I pushed your changeset here https://hg.savannah.gnu.org/hgweb/octave/rev/d2fd4616829b.

Only thing I did was add a line to the check-in message about which file and function were affected, and what the change was.  Octave's message standards are adopted from GNU and they include this extra level of detail.

Marking as fixed and closing report.

Rik <rik5>
Group administrator
Sat 20 Jun 2020 09:55:59 AM UTC, comment #20: 

Would one of the core devs please push (version from comment #18, file #49322)?

I tried from a stable branch copy but got "authorization failure".
Last time I pushed to the Octave repo must be several years ago; I'm probably doing something wrong but lack time to sort it out now.
Thanks

Philip Nienhuis <philipnienhuis>
Group Member
Fri 19 Jun 2020 07:02:43 PM UTC, comment #19: 

I think stable would be appropriate.  I am particularly swayed by the thought that there is no time frame for the 7.1 release--it might be six months, but it could also be 18 months--and we shouldn't wait that long to fix this.

Rik <rik5>
Group administrator
Fri 19 Jun 2020 06:07:24 PM UTC, comment #18: 

Just added the complete cset.

(file #49322)

Philip Nienhuis <philipnienhuis>
Group Member
Fri 19 Jun 2020 05:58:53 PM UTC, comment #17: 

One more question:
Push to stable or to default?

I'd say stable because:

  • This bug report originates from the time of the 4.x.x branch
  • Octave's Java code has been doing the incorrect thing since several years
  • It'll be quite some time before Octave-7.1.0 will be released
  • The patch doesn't do any harm AFAICS with dev Octave and Octave-6.0.1


Other opinions?

Philip Nienhuis <philipnienhuis>
Group Member
Tue 16 Jun 2020 07:48:32 PM UTC, comment #16: 

I'd say yes, if you remove the code, remove the comment that describes it in the same change.

John W. Eaton <jwe>
Group administrator
Tue 16 Jun 2020 07:11:32 PM UTC, comment #15: 

Sure, but shall I also simply remove L.620 with it?
I'd rather do it all in one cset.

Philip Nienhuis <philipnienhuis>
Group Member
Tue 16 Jun 2020 06:51:26 PM UTC, comment #14: 

@Philip: Go ahead and push it since the patch has had some review now by people who understand Java better than I do.

Rik <rik5>
Group administrator
Tue 16 Jun 2020 08:52:04 AM UTC, comment #13: 


> So do you think it is advisable to apply my patch in comment #3?


Yes. I think that is the correct behavior for Octave, and unlikely to break things.

Andrew Janke <apjanke>
Tue 16 Jun 2020 08:49:43 AM UTC, comment #12: 

@Andrew:
So do you think it is advisable to apply my patch in comment #3?

I have that patch since over a month in Linux & Windows w/o any issues or changes in program behaviour.

Philip Nienhuis <philipnienhuis>
Group Member
Tue 16 Jun 2020 01:26:51 AM UTC, comment #11: 

Java programmer here.

>  I think it is probably safe to make OctClassLoader not be the default class loader, since it is wrapped by ClassHelper anyway.


I think you're correct here. I think you only need to change the default/system class loader if you want the ability to intercept the loading of classes in the Java Standard Library or JRE internals, and Octave doesn't want to do that, and this option was here to work around an old Sun JVM bug like Shane says and is probably no longer needed, since most users are probably running newer JREs.

Andrew Janke <apjanke>
Fri 08 May 2020 11:11:04 AM UTC, comment #10: 

Apologies for quite a long post. I just want to document what I did to investigate the safety of my patch.

First off, 'make check' shows no FAILs with the patch.

As to "slowness", it turns out there's apparently some issue with LibreOffice, that is, its UNO-Java bridge that the io package invokes. Maybe it is just my system.

With and without patch makes for little or no performance difference. But I still don't know how representative it is what I did.

My procedure:

(install io-2.6.1 package). Then:

>> pkg prefix ~/octave6 ~/octave6  ## just to not interfere with my 7.0.0 installation
ans = /home/philip/octave6
>> pkg local_list ~/.octave6_packages; ## Needs patch from bug #58332
>> pkg local_list
/home/philip/.octave6_packages
>> pkg load io

## Amend following path to LibreOffice on your system:
chk_spreadsheet_support ("", 3, "/opt/libreoffice6.4")
;
; <long listing>
:
UNO interface supported now.

ans = 128
>> javaclasspath
   STATIC JAVA PATH

      - empty -

   DYNAMIC JAVA PATH

      /opt/libreoffice6.4/program/classes/ridl.jar
      /opt/libreoffice6.4/program/classes/unoloader.jar
      /opt/libreoffice6.4/program/classes/juh.jar
      /opt/libreoffice6.4/program/classes/jurt.jar
      /opt/libreoffice6.4/program/classes/unoil.jar
      /opt/libreoffice6.4/program

>> ts = cputime; tic; io_testscript ("UNO", "io-test.xlsx"); tt = cputime; printf ("cpu time = %f\n", tt - ts); toc

Testing .xls interface UNO using file io-test.xlsx...

 1. Initialize arrays...
 2. Insert first empty sheet...
 3. Add data to test sheet...
 4. Add another sheet with just one number in A1...
 5. Explore sheet info...
 6. Read data back...
 7. Tests part 1 (basic I/O):
    ...Numeric array... matches...
    ...Cellstr array... matches...
    ...special characters... matches...
    ...Boolean... returned as numeric '1' rather than logical TRUE.
 8. Repeat reading, now return formulas as text...
 9. Tests part 2 (read back formula):
    ...OK, formula recovered ('=C2+D2').
10. Cleaning up..... OK
cpu time = 1.742070
Elapsed time is 114.236 seconds.
>>


and CPU time and elapsed time vary from .81 - 1.75 and 50 - 150 seconds, resp.

I don't know why LibreOffice (through the Java UNO bridge) is so exruciatingly slow on my Linux system, on Windows it's > 20 times faster.
On the whole, taking into account the wide range of CPU and Elapsed time, I see little or no difference with and without the patch.

W.r.t. the other Java based spreadsheet I/O interfaces there's no significant difference either; the same tests run within a few seconds Elapsed time for all of them together:


## <following up from above verbatim block>
## unload previously loaded class libs:
>> chk_spreadsheet_support ("", -1)
ans = 0
>> chk_spreadsheet_support (tilde_expand ("~/Java"), 0)
ans = 126
>> javaclasspath
   STATIC JAVA PATH

      - empty -

   DYNAMIC JAVA PATH

      /home/philip/Java/jOpenDocument-1.4rc2.jar
      /home/philip/Java/xml-apis.jar
      /home/philip/Java/xercesImpl-2.11.0.jar
      /home/philip/Java/odfdom-java-0.8.8-incubating.jar
      /home/philip/Java/gwt-servlet-deps.jar
      /home/philip/Java/OpenXLS-10.jar
      /home/philip/Java/jxl.jar
      /home/philip/Java/commons-collections4-4.1.jar
      /home/philip/Java/dom4j-1.6.1.jar
      /home/philip/Java/poi-ooxml-schemas-3.17.jar
      /home/philip/Java/xmlbeans-2.6.0.jar
      /home/philip/Java/poi-ooxml-3.17.jar
      /home/philip/Java/poi-3.17.jar
>> ts = cputime; tic; test_spsh; tt = cputime; printf ("cpu time = %f\n", tt - ts); toc
:
: <even longer output listing>
:
- End of test_spsh -
cpu time = 8.467224
Elapsed time is 8.37337 seconds.  ## ?? smaller nr. than CPU time ??
>>


and those execution times are consistently around 8.4 seconds for each run, with or without the patch.

==> As far as I'm concerned the patch should be safe to apply.
Any opinions from other Java users ?

Philip Nienhuis <philipnienhuis>
Group Member
Fri 08 May 2020 07:28:40 AM UTC, comment #9: 

Before trying to apply my patch, let me first finish investigating apparent slowness on Linux as I outlined in comment #6 (I got severely distracted there but I'm back on track now)

Philip Nienhuis <philipnienhuis>
Group Member
Fri 08 May 2020 06:25:23 AM UTC, comment #8: 

I adapted the Octave Forge java package so that it could be a part of core Octave, but I didn't write the code nor do I know anything about what the real intent was for using the -Djava.system.class.loader=XXX option.

John W. Eaton <jwe>
Group administrator
Fri 08 May 2020 05:17:19 AM UTC, comment #7: 

It appears that this flag has been passed to the JVM since Java support was first added to Octave in 2012 by JWE:

https://github.com/NexMirror/Octave/commit/47a9b9ca1239e9343c88e39905567fc206b285ea

Based on the comments in OctClassLoader.java, it seems this was mainly a workaround to a bug in an old Sun JVM:

      // Look dynamically into java.library.path, because Sun VM does
      // not do it (seems to cache initial java.library.path instead)

However, OctClassLoader is also used in ClassHelper.java, which is used more widely in ov-java.cc.  ClassHelper seems to be the piece that marshals Octave calls to Java, and it runs its own class loader.

https://github.com/NexMirror/Octave/blob/master/scripts/java/org/octave/ClassHelper.java

Therefore, although I'm not fully confident, I think it is probably safe to make OctClassLoader not be the default class loader, since it is wrapped by ClassHelper anyway.

A reasonable resolution to this ticket might be:

1. Apply Philip's patch
2. Fix the following documentation in ov-java.cc that mentions OctClassLoader:

https://github.com/NexMirror/Octave/blob/eda9e9f0f053da5a05b11a897f6f8043b70ae9c8/libinterp/octave-value/ov-java.cc#L620

Shane <sffc>
Thu 07 May 2020 09:44:27 PM UTC, comment #6: 

Agreed, Mike.

I did note some slow execution on Linux with invoking LibreOffice for spreadsheet I/O, but while investigating whether that was because of the patch I got stuck in a few pkg.m bugs. On Windows there's no performance difference AFAICS.

Philip Nienhuis <philipnienhuis>
Group Member
Wed 06 May 2020 09:13:12 PM UTC, comment #5: 

I think we need some Java expert to weigh in on what a class loader is, what purpose it serves, and why it was added to Octave in the first place.

Mike Miller <mtmiller>
Group Member
Wed 06 May 2020 08:45:05 PM UTC, comment #4: 

Reopening as a patch is submitted.

BTW I don't follow why this bug was closed with status "Invalid" - reading comment #1 I think "Won't fix" would have been more appropriate.

Philip Nienhuis <philipnienhuis>
Group Member
Wed 06 May 2020 08:13:53 PM UTC, comment #3: 

I didn't know about this bug report.

In bug #58266 I've submitted a patch (because I believed this was another manifestation of the bug there) that simply removes the "-Djava.system.class.loader=XXX" line in __ov_java__.cc . I'll attach the patch here (w. changed bug number).

While according to Rik Octave currently does it right, with this patch I couldn't discern any difference in behavior as regards Java (w. Oracle Java 10 and 8).
My test code, the io package, invokes a lot of Java calls when using external Java libraries for spreadsheet I/O and LibreOffice so that gives at least me some assurance that the patch won't hurt.


(file #49016)

Philip Nienhuis <philipnienhuis>
Group Member
Fri 03 Jan 2020 10:16:42 AM UTC, comment #2: 

The warning appears for me with Octave 5.1 on CentOS 8 with java-11-openjdk.  It appears when loading many popular packages, including data-smoothing, financial, io, optim, and statistics.  I plan to write code to suppress the warning from being propagated to users in Octave Online, but it would be nice if we knew a way to prevent it from happening in the first place.

Shane <sffc>
Sat 21 Sep 2019 12:32:38 AM UTC, comment #1: 

I looked through the code and the documentation for java.system.class.loader, and I can't see anything we are doing incorrectly.  Use of the "-Djava.system.class.loader=XXX" is the way to override the system class loader as far as I can tell.  Another consideration is that this warning seems to be issued only by OpenJDK, not the true Oracle VM.  So, I think it just exists and we have to live with it.

Rik <rik5>
Group administrator
Thu 06 Dec 2018 07:36:50 PM UTC, original submission:  

Initializing the JVM with OpenJDK 11 gets the following warning:


octave:1> __java_init__ ;
OpenJDK 64-Bit Server VM warning: Archived non-system classes are disabled because the java.system.class.loader property is specified (value = "org.octave.OctClassLoader"). To use archived non-system classes, this property must be not be set


I'm not sure what that means.

When Octave starts the JVM, it uses the flag "-Djava.system.class.loader=org.octave.OctClassLoader" (see https://hg.savannah.gnu.org/hgweb/octave/file/0a145a1600e3/libinterp/octave-value/ov-java.cc#l723 ) which I believe is required to support the dynamic class path (source https://hg.savannah.gnu.org/hgweb/octave/file/0a145a1600e3/scripts/java/org/octave/OctClassLoader.java )

This new error message comes from OpenJDK and the commit that added it is http://hg.openjdk.java.net/jdk/jdk/rev/840e26123940 However, I can't make much sense of it. I don't quite get what CDS is and the two issues mentioned on the commit message don't make much sense to me https://bugs.openjdk.java.net/browse/JDK-8193213 and https://bugs.openjdk.java.net/browse/JDK-8182731

Carnë Draug <carandraug>
Group Member

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #49322:  bug_55174_Java_OctClassLoader.cset added by philipnienhuis (1KiB - application/octet-stream)
file #49016:  Java_system_loader.patch added by philipnienhuis (872B - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by philipnienhuis
  • -email is unavailable- added by apjanke (Posted a comment)
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by saturn
  • -email is unavailable- added by sffc (Posted a comment)
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by carandraug (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 12 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2020-06-20 rik5 StatusPatch Reviewed Fixed
        Open/ClosedOpen Closed
    2020-06-20 philipnienhuis Carbon-Copy- Added philipnienhuis
    2020-06-19 philipnienhuis Attached File- Added bug_55174_Java_OctClassLoader.cset, #49322
    2020-06-16 rik5 StatusPatch Submitted Patch Reviewed
    2020-05-06 mtmiller Open/ClosedClosed Open
    2020-05-06 philipnienhuis StatusInvalid / Not an Octave Bug Patch Submitted
    2020-05-06 philipnienhuis Attached File- Added Java_system_loader.patch, #49016
    2020-05-06 saturn Carbon-Copy- Added saturn
    2019-09-21 rik5 StatusNone Invalid / Not an Octave Bug
        Open/ClosedOpen Closed
    2019-01-24 mtmiller CategoryNone Interpreter

    Back to the top

    Powered by Savane 3.13-3230.
    Corresponding source code