bugGNU Octave - Bugs: bug #61208, [MXE Octave] Start scripts should...

 
 

bug #61208: [MXE Octave] Start scripts should set OPENBLAS_NUM_THREADS

Submitted by:  Markus Mützel <mmuetzel>
Submitted on:  Thu 23 Sep 2021 12:17:07 PM UTC  
 
Category:  None Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Performance
Status:  Patch Submitted Assigned to:  None
Originator Name:  Open/Closed:  Open
Release:  dev Operating System:  Microsoft Windows

Add a New Comment (Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

( Jump to the original submission)

Tue 05 Oct 2021 06:26:47 PM UTC, comment #6: 

Oh, that must be my ignorance :-)

Why not change the comment into s/th like:

'If running with wscript.exe, calling "Exec" (needed to get nr. of CPUs) will flash a window for a split second.

([OT]  Remarkable how simple things like calling one stupid little command in the end yields relatively big patches like this, isn't it)

Philip Nienhuis <philipnienhuis>
Project Member
Mon 04 Oct 2021 07:24:33 AM UTC, comment #5: 

You are probably referring to this hunk of the patch:

diff -r 18ed05625431 -r ff1b3f3aaf6e installer-files/octave-firsttime.vbs
--- a/installer-files/octave-firsttime.vbs        Sun Sep 19 18:29:32 2021 +0200
+++ b/installer-files/octave-firsttime.vbs        Thu Sep 23 14:20:41 2021 +0200
@@ -2,11 +2,23 @@

Set wshShell = CreateObject( "WScript.Shell" )

+' If running with wscript.exe, "Exec" will flash a window for a split second.
+' Relaunch with cscript.exe which doesn't show that window.
+If InStr(1, WScript.FullName, "wscript.exe", vbTextCompare) > 0 Then
+  AllArgs = ""
+  Set wshArgs = WScript.Arguments
+  For I = 0 to wshArgs.Count - 1
+    AllArgs = AllArgs & " " & chr(34) & wshArgs(I) & chr(34)
+  Next
+  WScript.Quit wshShell.Run("cscript.exe """ & WScript.ScriptFullName & """ " & AllArgs, 0, True)
+End If
+
+
' get the directory that script resides in
Set fso = CreateObject("Scripting.FileSystemObject")
OctavePath = fso.GetParentFolderName(WScript.ScriptFullName)

-' ctavePath is now the root of the install folder, but for msys2,
+' OctavePath is now the root of the install folder. But for msys2,
' OctavePath should be OctavePath/mingw64 or OctavePath/ming32
MSysType = "MSYS"
MSysPath = OctavePath

I was trying to be as clear as possible with the comment: It is "Exec" that causes the flashing window for me. We did not call "Exec" before that patch. So no flashing windows before that patch for me.
This patch adds a command that calls "wmic" using "Exec". That would cause the flashing window (for me) unless the hunk above is also added.

Do who have suggestions how to make that comment more easily understandable?

Markus Mützel <mmuetzel>
Project Member
Sun 03 Oct 2021 08:40:40 PM UTC, comment #4: 

No the last years I didn't note a cmd windows flashing by when starting Octave. IIRC the .vbs start script eliminated it, and/or JWE did something a few years back so that it got silenced.
TBH I wondered why you added that part of the patch in the first place, I figured because you did see it, or someone else reported seeing it.
So, is it really needed then?

With Matlab the temporary cmd window is quite prominent, but AFAIR only with the prereleases and older releases (we still use r2014a very often as that's the last before the object-based graphics, and it's simply a bit faster); it lasts about 2-3 seconds on all Windows boxes I've been using the last years. The Matlab splash screen only shows up after that cmd window disappeared.

Indeed an Octave splash screen (IMO only for the GUI) would be a "nice to have".

Philip Nienhuis <philipnienhuis>
Project Member
Sun 03 Oct 2021 12:25:29 PM UTC, comment #3: 

Do you see a flashing cmd window when starting Octave before that patch?
That doesn't appear for me here.
I also don't see a cmd window when starting Matlab on the same machine.

If that is a change in behavior on some systems, I agree that that part should probably go in a separate (prior) patch.

I was wondering whether we could add a splash screen that shows while Octave is loading (similar to what happens when starting Excel, Matlab or other programs that take some seconds to load). Preferably that would work cross-platform. (Maybe Qt has something ready-baked?)
But that should probably go in a dedicated report.

Markus Mützel <mmuetzel>
Project Member
Sat 02 Oct 2021 07:46:24 PM UTC, comment #2: 

I applied the patch manually in an existing Octave-7.0.0 installation on my desktop (core i5-4570 with 4 cores w/o hyperthreading) and on my laptop with an older core i5-3320 with 2 cores and hyperthreading.
The environment var gets set correctly to the actual number of cores.

The patch also removes the flashing cmd window (shouldn't that be  a separate patch, together with the typo fixes?).  Matlab r2021b still flashes a cmd window for quite a while, so in a way your patch removes that "Matlab compatibility" ;-)
Personally I'd like some start-up feedback but OK, the Octave GUI usually takes just a few seconds to start, in contrast to Matlab which takes 25 secs or so.

Philip Nienhuis <philipnienhuis>
Project Member
Thu 23 Sep 2021 12:21:57 PM UTC, comment #1: 

Please, see the attached patch.

(file #51953)

Markus Mützel <mmuetzel>
Project Member
Thu 23 Sep 2021 12:17:07 PM UTC, original submission:  

OpenBLAS seems to make errors and run less performant if the number of threads it uses exceeds the number of physical processor cores.
That seems to happen with default settings on machines with processor that use hyperthreading.
See also some posts interleaved in that discourse thread. E.g.:
https://octave.discourse.group/t/release-candidate-available/1227/21
https://octave.discourse.group/t/release-candidate-available/1227/117

I'll post a possible patch for the start scripts included in the Windows bundle created with MXE Octave as soon as I have a bug number.

Markus Mützel <mmuetzel>
Project Member

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #51953:  bug61208-mxe-openblas-threads.patch added by mmuetzel (10KiB - 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 mmuetzel (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only project members can vote.

     

     

     

    Follow 2 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2021-09-23 mmuetzel Attached File- => Added bug61208-mxe-openblas-threads.patch, #51953
        StatusNone => Patch Submitted

    Back to the top


    Powered by Savane 3.7