[BUG] SD card navigation is unusably slow

dbuezas commented 4 months ago

Did you test the latest `bugfix-2.1.x` code?

Yes, and the problem still exists.

Bug Description

When navigating the SD card from the UI (SSD1309), the UI becomes unusably slow even though I have a very speedy mcu (550MHz, skr3).

The problem is not there when I use SDCARD_SORT_ALPHA, but I wish the latest files would show at the top instead of the files being sorted by name

#define SDCARD_SORT_ALPHA

  // SD Card Sorting options
  #if ENABLED(SDCARD_SORT_ALPHA)
    #define SDSORT_REVERSE     true  // Default to sorting file names in reverse order.
    #define SDSORT_LIMIT       256     // Maximum number of sorted items (10-256). Costs 27 bytes each.
    #define SDSORT_FOLDERS     -1     // -1=above  0=none  1=below
    #define SDSORT_GCODE       false  // Enable G-code M34 to set sorting behaviors: M34 S<-1|0|1> F<-1|0|1>
    #define SDSORT_USES_RAM    true   // Pre-allocate a static array for faster pre-sorting.
    #define SDSORT_USES_STACK  false  // Prefer the stack for pre-sorting to give back some SRAM. (Negated by next 2 options.)
    #define SDSORT_CACHE_NAMES true   // Keep sorted items in RAM longer for speedy performance. Most expensive option.
    #define SDSORT_DYNAMIC_RAM false  // Use dynamic allocation (within SD menus). Least expensive option. Set SDSORT_LIMIT before use!
    #define SDSORT_CACHE_VFATS 5      // Maximum number of 13-byte VFAT entries to use for sorting.
                                      // Note: Only affects SCROLL_LONG_FILENAMES with SDSORT_CACHE_NAMES but not SDSORT_DYNAMIC_RAM.
  #endif

  // Allow international symbols in long filenames. To display correctly, the
  // LCD's font must contain the characters. Check your selected LCD language.
  #define UTF_FILENAME_SUPPORT

  #define LONG_FILENAME_HOST_SUPPORT    // Get the long filename of a file/folder with 'M33 <dosname>' and list long filenames with 'M20 L'
  #define LONG_FILENAME_WRITE_SUPPORT   // Create / delete files with long filenames via M28, M30, and Binary Transfer Protocol
  #define M20_TIMESTAMP_SUPPORT         // Include timestamps by adding the 'T' flag to M20 commands

  #define SCROLL_LONG_FILENAMES         // Scroll long filenames in the SD card menu

Bug Timeline

At least since Nov 2023, possibly since forever?

Expected behavior

The UI navigates files fast (possibly by caching them like with SDCARD_SORT_ALPHA but without sorting them)

Actual behavior

UI unusably slow while navigating sd card without SDCARD_SORT_ALPHA

Steps to Reproduce

No response

Version of Marlin Firmware

Latest bugfix 2.1.x

Printer model

UM2

Electronics

SKR3

LCD/Controller

SSD1309

Other add-ons

No response

Bed Leveling

ABL Bilinear mesh

Your Slicer

Other (explain below)

Host Software

None

Don't forget to include

[X] A ZIP file containing your Configuration.h and Configuration_adv.h.

Additional information & file uploads

Archive.zip

dbuezas commented 4 months ago

I hacked this together to keep the file list cache without actually sorting by name. BTW, any reason to use bubble sort here? heap sort has better time complexity, also sorts in-place (i.e low ram impact) and is almost as simple

diff --git a/Marlin/Configuration_adv.h b/Marlin/Configuration_adv.h
index dd1f036cff..08c6a1ab71 100644
--- a/Marlin/Configuration_adv.h
+++ b/Marlin/Configuration_adv.h
@@ -1797,6 +1797,7 @@

   // SD Card Sorting options
   #if ENABLED(SDCARD_SORT_ALPHA)
+    #define SDSORT_NO_SORT     true  // Keep cache but dont actually sort
     #define SDSORT_REVERSE     true  // Default to sorting file names in reverse order.
     #define SDSORT_LIMIT       256     // Maximum number of sorted items (10-256). Costs 27 bytes each.
     #define SDSORT_FOLDERS     -1     // -1=above  0=none  1=below
diff --git a/Marlin/src/sd/cardreader.cpp b/Marlin/src/sd/cardreader.cpp
index dadb1bcefc..de7fca22b8 100644
--- a/Marlin/src/sd/cardreader.cpp
+++ b/Marlin/src/sd/cardreader.cpp
@@ -1262,7 +1262,7 @@ void CardReader::cdroot() {

         // Init sort order.
         for (int16_t i = 0; i < fileCnt; i++) {
-          sort_order[i] = i;
+          sort_order[i] = (SDSORT_REVERSE && SDSORT_NO_SORT) ? fileCnt - i - 1: i;
           // If using RAM then read all filenames now.
           #if ENABLED(SDSORT_USES_RAM)
             selectFileByIndex(i);
@@ -1281,6 +1281,7 @@ void CardReader::cdroot() {

         // Bubble Sort
         for (int16_t i = fileCnt; --i;) {
+          TERN1(SDSORT_NO_SORT, break;)
           bool didSwap = false;
           int16_t o1 = sort_order[0];
           #if DISABLED(SDSORT_USES_RAM)

github-actions[bot] commented 1 month ago

Greetings from the Marlin AutoBot! This issue has had no activity for the last 90 days. Do you still see this issue with the latest bugfix-2.1.x code? Please add a reply within 14 days or this issue will be automatically closed. To keep a confirmed issue open we can also add a "Bug: Confirmed" tag.

Disclaimer: This is an open community project with lots of activity and limited resources. The main project contributors will do a bug sweep ahead of the next release, but any skilled member of the community may jump in at any time to fix this issue. That can take a while depending on our busy lives so please be patient, and take advantage of other resources such as the MarlinFirmware Discord to help solve the issue.

dbuezas commented 1 month ago

I still have the issue. Anybody else too? Otherwise it may be something specific to my setup?

piotr-go commented 1 month ago

I have similar problem. Slow SD navigation when "SDCARD_RATHERRECENTFIRST" enabled and "SDCARD_SORT_ALPHA" disabled. Your patch worked. Thank You.

Marlin-2.1.2.4 BigTreeTech SKR Mini E3 1.2

MarlinFirmware / Marlin