bugGNU Octave - Bugs: bug #57459, Dynamically loaded SuiteSparse...

 
 

bug #57459: Dynamically loaded SuiteSparse functions should be moved to core

Submitter:  Markus Mützel <mmuetzel>
Submitted:  Sun 22 Dec 2019 08:49:05 AM UTC
   
 
Category:  Interpreter Severity:  2 - Minor
Priority:  5 - Normal Item Group:  Other
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * dev Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Fri 08 May 2020 08:47:47 PM UTC, comment #10: 

It is still open if we should reset the settings in the persistent and shared SuiteSparse_config structure after use.
But that is probably a better topic for another bug report.

Closing this bug as fixed.

Markus Mützel <mmuetzel>
Group administrator
Fri 08 May 2020 07:41:11 PM UTC, comment #9: 

Is this bug complete? From the discussion seems like it was resolved a few months ago.

Mike Miller <mtmiller>
Group Member
Sun 02 Feb 2020 10:16:00 PM UTC, comment #8: 

Just to verify, same results (no errors) as comment #7 here on Windows with dev Octave.

Philip Nienhuis <philipnienhuis>
Group Member
Sun 02 Feb 2020 05:34:22 PM UTC, comment #7: 

Thanks. With your change, cross-compilation works for me. And the buildbots seem to be happy, too.
Also, the test suite completes without unexpected errors on Windows.

Markus Mützel <mmuetzel>
Group administrator
Fri 31 Jan 2020 02:48:58 PM UTC, comment #6: 

I pushed the following changeset for the linking issue.  I haven't tested it but I think it should fix the problem on Windows.  We'll see if the buildbots approve.

http://hg.savannah.gnu.org/hgweb/octave/rev/ef9a4e056c78

John W. Eaton <jwe>
Group administrator
Thu 30 Jan 2020 09:16:26 PM UTC, comment #5: 

MXE Octave cross builds for Windows are failing with an error that is probably related to this change. The linker complains about a bunch of missing symbols that all seem to be related to SuiteSparse.
I'm attaching the build log.

(file #48308)

Markus Mützel <mmuetzel>
Group administrator
Thu 30 Jan 2020 08:03:33 AM UTC, comment #4: 

I think I get your point now.
I don't know how those SuiteSparse_config settings are shared. Does Octave own its own settings structure or is that structure shared among different programs using SuiteSparse functions (i.e. are those settings thread local or global)?
In any case, I agree that it would be safest to set the pointers each time we call a SuiteSparse function that uses them. If we want to be nice to other functions using SuiteSparse with different settings, we might want to reset them to the previous values once we are done.
In the latter case (SuiteSparse_config share between threads/programs), I'm not sure how that could be done in a thread-safe manner.

Markus Mützel <mmuetzel>
Group administrator
Wed 29 Jan 2020 06:53:43 PM UTC, comment #3: 

These pointers are set in a number of other functions.

I'm now thinking that we should set them each time we need them to be set for our purposes and reset them to the previous value when done.  That way we don't cause trouble for other code that may also be using these pointers.  If we just set them once when liboctave is initialized, then some other function (not our code) may wipe out our setting.

John W. Eaton <jwe>
Group administrator
Wed 29 Jan 2020 02:55:31 PM UTC, comment #2: 

I'm not sure either if the function pointers should be reset.
IIUC, the settings were persistent and shared before this change, and it seems to have worked. I don't know why it would start failing now these functions are no longer dynamically loaded (but live in core Octave). So I guess we don't need to reset them.

Markus Mützel <mmuetzel>
Group administrator
Wed 29 Jan 2020 01:35:17 PM UTC, comment #1: 

I pushed the following change on default:

http://hg.savannah.gnu.org/hgweb/octave/rev/c28b8ba841fb

Now I'm not sure about what should happen with the function pointers that are set in amd and symbfact.  Are those supposed to be set temporarily, for the duration of functions that call SuiteSparse functions, or once when the application starts?  If they are supposed to be set temporarily, then we should probably be using unwind_protect objects to reset them.

John W. Eaton <jwe>
Group administrator
Sun 22 Dec 2019 08:49:05 AM UTC, original submission:  

See jwe's comment #19 in bug #57435:

> If locking amd.oct avoids the problem, then maybe we should just move that function to corefcn instead of defining it as a .oct file in dldfcn.  The suitesparse libraries are always linked with liboctave if they are available so there is no real point in dynamically loading functions that depend on that library.
>
> If we always load suitesparse libraries if they are available, then we could also refactor the way that suitesparse config is handled as the comments in SuiteSparse_config.h recommend.  Instead of attempting to initialize those function pointers on demand, we could do it once at startup.


Markus Mützel <mmuetzel>
Group administrator

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #48308:  default-octave.7z added by mmuetzel (128KiB - 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 jwe (Posted a comment)
  • -email is unavailable- added by mmuetzel (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 3 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2020-05-08 mmuetzel StatusNone Fixed
        Open/ClosedOpen Closed
    2020-01-30 mmuetzel Attached File- Added default-octave.7z, #48308

    Back to the top

    Powered by Savane 3.13-4448.
    Corresponding source code