Browse Source

Fast skipping of large comment blocks

This is an extension/optimization of PR #2956.
It uses the cached 512B block buffer to avoid heavy-weight read() in SdBaseFile.
Even though this principle allowed the AVR to skip ~600KB of data within ~5 seconds,
the impact on code base is huge, especially into well proven and long-term stable
parts like reading a file from the SD card.

The sole purpose of this PR is to show/verify the possibility of the AVR CPU
in relation to adding thumbnails into MK3 G-codes.
Moreover, this PR shall not be merged unless the missing/commented features
are restored - especially file seeking and M84 search.

PFW-1175
D.R.racer 3 years ago
parent
commit
c3758d350e

+ 4 - 2
Firmware/Marlin_main.cpp

@@ -3987,7 +3987,9 @@ void process_commands()
 #endif
 		}else if (code_seen_P("fv")) { // PRUSA fv
         // get file version
-        #ifdef SDSUPPORT
+        #if 0 
+            //@@TODO
+            def SDSUPPORT
         card.openFile(strchr_pointer + 3,true);
         while (true) {
             uint16_t readByte = card.get();
@@ -5767,7 +5769,7 @@ if(eSoundMode!=e_SOUND_MODE_SILENT)
       starpos = (strchr(strchr_pointer + 4,'*'));
 	  if(starpos!=NULL)
         *(starpos)='\0';
-      card.openFile(strchr_pointer + 4,true);
+      card.openFileFilteredGcode(strchr_pointer + 4);
       break;
 
     /*!

+ 122 - 4
Firmware/SdBaseFile.cpp

@@ -530,9 +530,21 @@ bool SdBaseFile::mkdir(SdBaseFile* parent, const uint8_t dname[11]) {
   * \return The value one, true, is returned for success and
   * the value zero, false, is returned for failure.
   */
-  bool SdBaseFile::open(const char* path, uint8_t oflag) {
-    return open(cwd_, path, oflag);
-  }
+bool SdBaseFile::open(const char* path, uint8_t oflag) {
+  return open(cwd_, path, oflag);
+}
+  
+bool SdBaseFile::openFilteredGcode(SdBaseFile* dirFile, const char* path){
+    if( open(dirFile, path, O_READ) ){
+        gf.reset(0,0);
+        // compute the block to start with
+        if( ! computeNextFileBlock(&gf.block, &gf.offset) )
+            return false;
+        return true;
+    } else {
+        return false;
+    }
+}
 //------------------------------------------------------------------------------
 /** Open a file or directory by name.
  *
@@ -1030,6 +1042,112 @@ int16_t SdBaseFile::read() {
   uint8_t b;
   return read(&b, 1) == 1 ? b : -1;
 }
+
+int16_t SdBaseFile::readFilteredGcode() {
+    // avoid calling the default heavy-weight read() for just one byte
+    return gf.read_byte();
+}
+
+void GCodeInputFilter::reset(uint32_t blk, uint16_t ofs){
+    // @@TODO clean up
+    block = blk;
+    offset = ofs;
+    cachePBegin = sd->vol_->cache()->data;
+    // reset cache read ptr to its begin
+    cacheP = cachePBegin;
+}
+
+int16_t GCodeInputFilter::read_byte(){
+    EnsureBlock(); // this is unfortunate :( ... other calls are using the cache and we can loose the data block of our gcode file
+
+    // assume, we have the 512B block cache filled and terminated with a '\n'
+//    SERIAL_PROTOCOLPGM("read_byte enter:");
+//    for(uint8_t i = 0; i < 16; ++i){
+//        SERIAL_PROTOCOL( cacheP[i] );
+//    }
+    
+    const uint8_t *start = cacheP;
+    uint8_t consecutiveCommentLines = 0;
+    while( *cacheP == ';' ){
+        for(;;){
+            while( *(++cacheP) != '\n' ); // skip until a newline is found
+            // found a newline, prepare the next block if block cache end reached
+            if( cacheP - cachePBegin >= 512 ){
+                // at the end of block cache, fill new data in
+                sd->curPosition_ += cacheP - start;
+                if( ! sd->computeNextFileBlock(&block, &offset) )goto fail;
+                EnsureBlock(); // fetch it into RAM
+                cacheP = start = cachePBegin;
+            } else {
+                if(++consecutiveCommentLines == 255){
+                    // SERIAL_PROTOCOLLN(sd->curPosition_);
+                    goto forceExit;
+                }
+                // peek the next byte - we are inside the block at least at 511th index - still safe
+                if( *(cacheP+1) == ';' ){
+                    // consecutive comment
+                    ++cacheP;
+                    ++consecutiveCommentLines;
+                }
+                break; // found the real end of the line even across many blocks
+            }
+        }
+    }
+forceExit:
+    sd->curPosition_ += cacheP - start + 1;
+    {
+    int16_t rv = *cacheP++;
+    
+    // prepare next block if needed
+    if( cacheP - cachePBegin >= 512 ){
+//        SERIAL_PROTOCOLLN(sd->curPosition_);
+        if( ! sd->computeNextFileBlock(&block, &offset) )goto fail;
+        // don't need to force fetch the block here, it will get loaded on the next call
+        cacheP = cachePBegin;
+    }    
+    return rv;
+    }
+fail:
+//    SERIAL_PROTOCOLLNPGM("CacheFAIL");
+    return -1;
+}
+
+bool GCodeInputFilter::EnsureBlock(){
+    if ( sd->vol_->cacheRawBlock(block, SdVolume::CACHE_FOR_READ)){
+        // terminate with a '\n'
+        const uint16_t terminateOfs = (sd->fileSize_ - offset) < 512 ? (sd->fileSize_ - offset) : 512;
+        sd->vol_->cache()->data[ terminateOfs ] = '\n';
+        return true;
+    } else {
+        return false;
+    }
+}
+
+bool SdBaseFile::computeNextFileBlock(uint32_t *block, uint16_t *offset) {
+    // error if not open or write only
+    if (!isOpen() || !(flags_ & O_READ)) return false;
+
+    *offset = curPosition_ & 0X1FF;  // offset in block
+    if (type_ == FAT_FILE_TYPE_ROOT_FIXED) {
+        *block = vol_->rootDirStart() + (curPosition_ >> 9);
+    } else {
+        uint8_t blockOfCluster = vol_->blockOfCluster(curPosition_);
+        if (*offset == 0 && blockOfCluster == 0) {
+            // start of new cluster
+            if (curPosition_ == 0) {
+                // use first cluster in file
+                curCluster_ = firstCluster_;
+            } else {
+                // get next cluster from FAT
+                if (!vol_->fatGet(curCluster_, &curCluster_)) return false;
+            }
+        }
+        *block = vol_->clusterStartBlock(curCluster_) + blockOfCluster;
+    }
+    return true;
+}
+
+
 //------------------------------------------------------------------------------
 /** Read data from a file starting at the current position.
  *
@@ -1443,7 +1561,7 @@ bool SdBaseFile::rmRfStar() {
  * \param[in] oflag Values for \a oflag are constructed by a bitwise-inclusive
  * OR of open flags. see SdBaseFile::open(SdBaseFile*, const char*, uint8_t).
  */
-SdBaseFile::SdBaseFile(const char* path, uint8_t oflag) {
+SdBaseFile::SdBaseFile(const char* path, uint8_t oflag):gf(this) {
   type_ = FAT_FILE_TYPE_CLOSED;
   writeError = false;
   open(path, oflag);

+ 28 - 1
Firmware/SdBaseFile.h

@@ -174,15 +174,34 @@ static inline uint8_t FAT_SECOND(uint16_t fatTime) {
 uint16_t const FAT_DEFAULT_DATE = ((2000 - 1980) << 9) | (1 << 5) | 1;
 /** Default time for file timestamp is 1 am */
 uint16_t const FAT_DEFAULT_TIME = (1 << 11);
+
+
+class SdBaseFile;
+class GCodeInputFilter {
+    SdBaseFile *sd; //@@TODO subject to removal - merge with some derived SdFileXX
+    const uint8_t *cachePBegin;
+    const uint8_t *cacheP;
+    bool EnsureBlock();
+public:
+    uint32_t block; // remember the current file block to be kept in cache - due to reuse of the memory, the block may fall out a must be read back
+    uint16_t offset;
+
+public:
+    inline GCodeInputFilter(SdBaseFile *sd):sd(sd){}
+    void reset(uint32_t blk, uint16_t ofs);
+    int16_t read_byte();
+};
+
 //------------------------------------------------------------------------------
 /**
  * \class SdBaseFile
  * \brief Base class for SdFile with Print and C++ streams.
  */
 class SdBaseFile {
+    GCodeInputFilter gf;
  public:
   /** Create an instance. */
-  SdBaseFile() : writeError(false), type_(FAT_FILE_TYPE_CLOSED) {}
+  SdBaseFile() : gf(this), writeError(false), type_(FAT_FILE_TYPE_CLOSED) {}
   SdBaseFile(const char* path, uint8_t oflag);
   ~SdBaseFile() {if(isOpen()) close();}
   /**
@@ -275,14 +294,21 @@ class SdBaseFile {
   bool open(SdBaseFile* dirFile, uint16_t index, uint8_t oflag);
   bool open(SdBaseFile* dirFile, const char* path, uint8_t oflag);
   bool open(const char* path, uint8_t oflag = O_READ);
+  bool openFilteredGcode(SdBaseFile* dirFile, const char* path);
   bool openNext(SdBaseFile* dirFile, uint8_t oflag);
   bool openRoot(SdVolume* vol);
   int peek();
   static void printFatDate(uint16_t fatDate);
   static void printFatTime( uint16_t fatTime);
   bool printName();
+  
+  int16_t readFilteredGcode();
+  bool computeNextFileBlock(uint32_t *block, uint16_t *offset);
+
+private:  
   int16_t read();
   int16_t read(void* buf, uint16_t nbyte);
+public:
   int8_t readDir(dir_t* dir, char* longFilename);
   static bool remove(SdBaseFile* dirFile, const char* path);
   bool remove();
@@ -322,6 +348,7 @@ class SdBaseFile {
   int16_t write(const void* buf, uint16_t nbyte);
 //------------------------------------------------------------------------------
  private:
+  friend class GCodeInputFilter;
   // allow SdFat to set cwd_
   friend class SdFat;
   // global pointer to cwd dir

+ 3 - 2
Firmware/SdVolume.h

@@ -36,7 +36,7 @@
  */
 union cache_t {
            /** Used to access cached file data blocks. */
-  uint8_t  data[512];
+  uint8_t  data[512 + 1]; // abuse the last byte for saving '\n' - ugly optimization of read_filtered's inner skipping loop
            /** Used to access cached FAT16 entries. */
   uint16_t fat16[256];
            /** Used to access cached FAT32 entries. */
@@ -119,6 +119,7 @@ class SdVolume {
   bool dbgFat(uint32_t n, uint32_t* v) {return fatGet(n, v);}
 //------------------------------------------------------------------------------
  private:
+  friend class GCodeInputFilter;
   // Allow SdBaseFile access to SdVolume private data.
   friend class SdBaseFile;
 
@@ -211,4 +212,4 @@ class SdVolume {
 #endif  // ALLOW_DEPRECATED_FUNCTIONS
 };
 #endif  // SdVolume
-#endif
+#endif

+ 65 - 0
Firmware/cardreader.cpp

@@ -375,6 +375,71 @@ void CardReader::diveSubfolder (const char *fileName, SdFile& dir)
     }
 }
 
+// @@TODO merge with openFile, too much duplicated code and texts
+void CardReader::openFileFilteredGcode(const char* name, bool replace_current/* = false*/){
+    if(!cardOK)
+        return;
+    
+    if(file.isOpen()){  //replacing current file by new file, or subfile call
+        if(!replace_current){
+            if((int)file_subcall_ctr>(int)SD_PROCEDURE_DEPTH-1){
+                // SERIAL_ERROR_START;
+                // SERIAL_ERRORPGM("trying to call sub-gcode files with too many levels. MAX level is:");
+                // SERIAL_ERRORLN(SD_PROCEDURE_DEPTH);
+                kill(_n("trying to call sub-gcode files with too many levels."), 1);
+                return;
+            }
+            
+            SERIAL_ECHO_START;
+            SERIAL_ECHOPGM("SUBROUTINE CALL target:\"");
+            SERIAL_ECHO(name);
+            SERIAL_ECHOPGM("\" parent:\"");
+            
+            //store current filename and position
+            getAbsFilename(filenames[file_subcall_ctr]);
+            
+            SERIAL_ECHO(filenames[file_subcall_ctr]);
+            SERIAL_ECHOPGM("\" pos");
+            SERIAL_ECHOLN(sdpos);
+            filespos[file_subcall_ctr]=sdpos;
+            file_subcall_ctr++;
+        } else {
+            SERIAL_ECHO_START;
+            SERIAL_ECHOPGM("Now doing file: ");
+            SERIAL_ECHOLN(name);
+        }
+        file.close();
+    } else { //opening fresh file
+        file_subcall_ctr=0; //resetting procedure depth in case user cancels print while in procedure
+        SERIAL_ECHO_START;
+        SERIAL_ECHOPGM("Now fresh file: ");
+        SERIAL_ECHOLN(name);
+    }
+    sdprinting = false;
+  
+    SdFile myDir;
+    const char *fname=name;
+    diveSubfolder(fname,myDir);
+  
+    if (file.openFilteredGcode(curDir, fname)) {
+        filesize = file.fileSize();
+        SERIAL_PROTOCOLRPGM(_N("File opened: "));////MSG_SD_FILE_OPENED
+        SERIAL_PROTOCOL(fname);
+        SERIAL_PROTOCOLRPGM(_n(" Size: "));////MSG_SD_SIZE
+        SERIAL_PROTOCOLLN(filesize);
+        sdpos = 0;
+        
+        SERIAL_PROTOCOLLNRPGM(_N("File selected"));////MSG_SD_FILE_SELECTED
+        getfilename(0, fname);
+        lcd_setstatus(longFilename[0] ? longFilename : fname);
+        lcd_setstatus("SD-PRINTING         ");
+      } else {
+        SERIAL_PROTOCOLRPGM(MSG_SD_OPEN_FILE_FAIL);
+        SERIAL_PROTOCOL(fname);
+        SERIAL_PROTOCOLLN('.');
+      }
+}
+
 void CardReader::openFile(const char* name,bool read, bool replace_current/*=true*/)
 {
   if(!cardOK)

+ 5 - 1
Firmware/cardreader.h

@@ -20,6 +20,7 @@ public:
 
   void checkautostart(bool x); 
   void openFile(const char* name,bool read,bool replace_current=true);
+  void openFileFilteredGcode(const char* name, bool replace_current = false);
   void openLogFile(const char* name);
   void removeFile(const char* name);
   void closefile(bool store_location=false);
@@ -59,7 +60,10 @@ public:
 
   FORCE_INLINE bool isFileOpen() { return file.isOpen(); }
   FORCE_INLINE bool eof() { return sdpos>=filesize ;};
-  FORCE_INLINE int16_t get() {  sdpos = file.curPosition();return (int16_t)file.read();};
+//  FORCE_INLINE int16_t getX() {  sdpos = file.curPosition();return (int16_t)file.read();};
+  //@@TODO potential performance problem - when the comment reading fails, sdpos points to the last correctly read character.
+  // However, repeated reading (e.g. after power panic) the comment will be read again - it should survive correctly, it will just take a few moments to skip
+  FORCE_INLINE int16_t getFilteredGcodeChar() {  sdpos = file.curPosition();return (int16_t)file.readFilteredGcode();};
   FORCE_INLINE void setIndex(long index) {sdpos = index;file.seekSet(index);};
   FORCE_INLINE uint8_t percentDone(){if(!isFileOpen()) return 0; if(filesize) return sdpos/((filesize+99)/100); else return 0;};
   FORCE_INLINE char* getWorkDirName(){workDir.getFilename(filename);return filename;};

+ 13 - 11
Firmware/cmdqueue.cpp

@@ -573,7 +573,7 @@ void get_command()
   // this character _can_ occur in serial com, due to checksums. however, no checksums are used in SD printing
 
   static bool stop_buffering=false;
-  static uint8_t consecutiveEmptyLines = 0;
+//  static uint8_t consecutiveEmptyLines = 0;
   if(buflen==0) stop_buffering=false;
   union {
     struct {
@@ -585,11 +585,11 @@ void get_command()
   sd_count.value = 0;
   // Reads whole lines from the SD card. Never leaves a half-filled line in the cmdbuffer.
   while( !card.eof() && !stop_buffering) {
-    int16_t n=card.get();
+    int16_t n=card.getFilteredGcodeChar();
     char serial_char = (char)n;
     if( serial_char == '\n'
      || serial_char == '\r'
-     || ((serial_char == '#' || serial_char == ':') && comment_mode == false)
+     || ((serial_char == '#' || serial_char == ':') /*&& comment_mode == false*/)
      || serial_count >= (MAX_CMD_SIZE - 1)
      || n==-1
     ){
@@ -603,12 +603,12 @@ void get_command()
         // read from the sdcard into sd_count, 
         // so that the lenght of the already read empty lines and comments will be added
         // to the following non-empty line. 
-        comment_mode = false;
-        if( ++consecutiveEmptyLines > 250 ){
-            consecutiveEmptyLines = 0;
+//        comment_mode = false;
+//        if( ++consecutiveEmptyLines > 10 ){
+//            consecutiveEmptyLines = 0;
             return; // prevent cycling indefinitely - let manage_heaters do their job
-        }
-        continue; //if empty line
+//        }
+//        continue; //if empty line
       }
       // The new command buffer could be updated non-atomically, because it is not yet considered
       // to be inside the active queue.
@@ -644,7 +644,7 @@ void get_command()
 
       comment_mode = false; //for new command
       serial_count = 0; //clear buffer
-      consecutiveEmptyLines = 0; // reached a non-empty line which shall be enqueued
+//      consecutiveEmptyLines = 0; // reached a non-empty line which shall be enqueued
     
       if(card.eof()) break;
 
@@ -654,8 +654,10 @@ void get_command()
     }
     else
     {
-      if(serial_char == ';') comment_mode = true;
-      else if(!comment_mode) cmdbuffer[bufindw+CMDHDRSIZE+serial_count++] = serial_char;
+      /*if(serial_char == ';') comment_mode = true;
+      else if(!comment_mode)*/
+        // there are no comments coming from the filtered file
+        cmdbuffer[bufindw+CMDHDRSIZE+serial_count++] = serial_char;
     }
   }
   if(card.eof())

+ 3 - 1
Firmware/ultralcd.cpp

@@ -8473,8 +8473,10 @@ static void lcd_selftest_screen_step(int _row, int _col, int _state, const char
 /** Menu action functions **/
 
 static bool check_file(const char* filename) {
+    return true; // @@TODO
+    
 	if (farm_mode) return true;
-	card.openFile((char*)filename, true);
+	card.openFileFilteredGcode((char*)filename, true); //@@TODO
 	bool result = false;
 	const uint32_t filesize = card.getFileSize();
 	uint32_t startPos = 0;