From fe2fe0e2d8d211e589f5ceec0aa9ea65232aadb5 Mon Sep 17 00:00:00 2001 From: Gordon Williams Date: Wed, 13 Dec 2017 12:20:00 +0000 Subject: [PATCH] Filesystem API now uses flat strings (avoiding the 512 byte copy for each call) --- ChangeLog | 1 + libs/filesystem/jswrap_file.c | 91 +++++++++++++++-------------------- libs/filesystem/jswrap_file.h | 5 +- 3 files changed, 43 insertions(+), 54 deletions(-) diff --git a/ChangeLog b/ChangeLog index f0ded0632..ce359fe7a 100644 --- a/ChangeLog +++ b/ChangeLog @@ -39,6 +39,7 @@ Fix lexing of '/*/' as a complete block comment nRF5x: Add support for negating pins in software (eg. buttons/LEDs) Add `E.setFlags({unsyncFiles:1}` which doesn't sync the file to the SD card after each write - it's *much* faster + Filesystem API now uses flat strings (avoiding the 512 byte copy for each call) 1v94 : Allow Espruino boards to reset straight out of the DFU Bootloader Improvements in handling errors in code running in IRQs diff --git a/libs/filesystem/jswrap_file.c b/libs/filesystem/jswrap_file.c index 22f8a86eb..819c2f334 100755 --- a/libs/filesystem/jswrap_file.c +++ b/libs/filesystem/jswrap_file.c @@ -19,7 +19,6 @@ #define JS_FS_DATA_NAME JS_HIDDEN_CHAR_STR"FSd" // the data in each file #define JS_FS_OPEN_FILES_NAME JS_HIDDEN_CHAR_STR"FSo" // the list of open files - #if !defined(LINUX) && !defined(USE_FILESYSTEM_SDIO) && !defined(USE_FLASHFS) #define SD_CARD_ANYWHERE #endif @@ -169,28 +168,15 @@ static JsVar* fsGetArray(bool create) { static bool fileGetFromVar(JsFile *file, JsVar *parent) { bool ret = false; JsVar *fHandle = jsvObjectGetChild(parent, JS_FS_DATA_NAME, 0); - if (fHandle) { - jsvGetString(fHandle, (char*)&file->data, sizeof(JsFileData)+1/*trailing zero*/); - jsvUnLock(fHandle); + if (fHandle && jsvIsFlatString(fHandle)) { + file->data = jsvGetFlatStringPointer(fHandle); file->fileVar = parent; - if(file->data.state == FS_OPEN) {// return false if the file has been closed. + if(file->data->state == FS_OPEN) {// return false if the file has been closed. ret = true; } } - return ret; -} - -static void fileSetVar(JsFile *file) { - JsVar *fHandle = jsvFindChildFromString(file->fileVar, JS_FS_DATA_NAME, true); - JsVar *data = jsvSkipName(fHandle); - if (!data) { - data = jsvNewStringOfLength(sizeof(JsFileData), NULL); - jsvSetValueOfName(fHandle, data); - } jsvUnLock(fHandle); - assert(data); - jsvSetString(data, (char*)&file->data, sizeof(JsFileData)); - jsvUnLock(data); + return ret; } /*JSON{ @@ -239,10 +225,19 @@ void jswrap_E_unmountSD() { static bool allocateJsFile(JsFile* file,FileMode mode, FileType type) { JsVar *parent = jspNewObject(0, "File"); if (!parent) return false; // low memory + + JsVar *data = jsvNewFlatStringOfLength(sizeof(JsFileData)); + if (!data) { // out of memory for flat string + jsvUnLock(parent); + return false; + } + file->data = jsvGetFlatStringPointer(data); + jsvObjectSetChildAndUnLock(parent, JS_FS_DATA_NAME, data); file->fileVar = parent; - file->data.mode = mode; - file->data.type = type; - file->data.state = FS_NONE; + assert(file->data); + file->data->mode = mode; + file->data->type = type; + file->data->state = FS_NONE; return true; } @@ -263,6 +258,7 @@ Open a file JsVar *jswrap_E_openFile(JsVar* path, JsVar* mode) { FRESULT res = FR_INVALID_NAME; JsFile file; + file.data = 0; file.fileVar = 0; FileMode fMode = FM_NONE; if (jsfsInit()) { @@ -309,15 +305,14 @@ JsVar *jswrap_E_openFile(JsVar* path, JsVar* mode) { } if(fMode != FM_NONE && allocateJsFile(&file, fMode, FT_FILE)) { #ifndef LINUX - if ((res=f_open(&file.data.handle, pathStr, ff_mode)) == FR_OK) { - if (append) f_lseek(&file.data.handle, file.data.handle.fsize); // move to end of file + if ((res=f_open(&file.data->handle, pathStr, ff_mode)) == FR_OK) { + if (append) f_lseek(&file.data->handle, file.data->handle.fsize); // move to end of file #else - file.data.handle = fopen(pathStr, modeStr); - if (file.data.handle) { + file.data->handle = fopen(pathStr, modeStr); + if (file.data->handle) { res=FR_OK; #endif - file.data.state = FS_OPEN; - fileSetVar(&file); + file.data->state = FS_OPEN; // add to list of open files jsvArrayPush(arr, file.fileVar); } else { @@ -352,15 +347,14 @@ Close an open file. void jswrap_file_close(JsVar* parent) { if (jsfsInit()) { JsFile file; - if (fileGetFromVar(&file, parent) && file.data.state == FS_OPEN) { + if (fileGetFromVar(&file, parent) && file.data->state == FS_OPEN) { #ifndef LINUX - f_close(&file.data.handle); + f_close(&file.data->handle); #else - fclose(file.data.handle); - file.data.handle = 0; + fclose(file.data->handle); + file.data->handle = 0; #endif - file.data.state = FS_CLOSED; - fileSetVar(&file); + file.data->state = FS_CLOSED; // TODO: could try and free the memory used by file.data ? JsVar *arr = fsGetArray(false); @@ -401,7 +395,7 @@ size_t jswrap_file_write(JsVar* parent, JsVar* buffer) { if (jsfsInit()) { JsFile file; if (fileGetFromVar(&file, parent)) { - if(file.data.mode == FM_WRITE || file.data.mode == FM_READ_WRITE) { + if(file.data->mode == FM_WRITE || file.data->mode == FM_READ_WRITE) { JsvIterator it; jsvIteratorNew(&it, buffer, JSIF_EVERY_ARRAY_ELEMENT); char buf[32]; @@ -416,9 +410,9 @@ size_t jswrap_file_write(JsVar* parent, JsVar* buffer) { // write it out size_t written = 0; #ifndef LINUX - res = f_write(&file.data.handle, &buf, n, &written); + res = f_write(&file.data->handle, &buf, n, &written); #else - written = fwrite(&buf, 1, n, file.data.handle); + written = fwrite(&buf, 1, n, file.data->handle); #endif bytesWritten += written; if(written == 0) @@ -429,14 +423,12 @@ size_t jswrap_file_write(JsVar* parent, JsVar* buffer) { // finally, sync - just in case there's a reset or something if (!jsfGetFlag(JSF_UNSYNC_FILES)) { #ifndef LINUX - f_sync(&file.data.handle); + f_sync(&file.data->handle); #else - fflush(file.data.handle); + fflush(file.data->handle); #endif } } - - fileSetVar(&file); } } @@ -467,20 +459,19 @@ JsVar *jswrap_file_read(JsVar* parent, int length) { if (jsfsInit()) { JsFile file; if (fileGetFromVar(&file, parent)) { - if(file.data.mode == FM_READ || file.data.mode == FM_READ_WRITE) { + if(file.data->mode == FM_READ || file.data->mode == FM_READ_WRITE) { size_t actual = 0; #ifndef LINUX // if we're able to load this into a flat string, do it! - size_t len = f_size(&file.data.handle)-f_tell(&file.data.handle); + size_t len = f_size(&file.data->handle)-f_tell(&file.data->handle); if ( len == 0 ) { // file all read return 0; // if called from a pipe signal end callback } if (len > (size_t)length) len = (size_t)length; buffer = jsvNewFlatStringOfLength(len); if (buffer) { - res = f_read(&file.data.handle, jsvGetFlatStringPointer(buffer), len, &actual); + res = f_read(&file.data->handle, jsvGetFlatStringPointer(buffer), len, &actual); if (res) jsfsReportError("Unable to read file", res); - fileSetVar(&file); return buffer; } #endif @@ -492,10 +483,10 @@ JsVar *jswrap_file_read(JsVar* parent, int length) { requested = sizeof( buf ); actual = 0; #ifndef LINUX - res = f_read(&file.data.handle, buf, requested, &actual); + res = f_read(&file.data->handle, buf, requested, &actual); if(res) break; #else - actual = fread(buf, 1, requested, file.data.handle); + actual = fread(buf, 1, requested, file.data->handle); #endif if (actual>0) { if (!buffer) { @@ -510,7 +501,6 @@ JsVar *jswrap_file_read(JsVar* parent, int length) { bytesRead += actual; if(actual != requested) break; } - fileSetVar(&file); } } } @@ -556,13 +546,12 @@ void jswrap_file_skip_or_seek(JsVar* parent, int nBytes, bool is_skip) { if (jsfsInit()) { JsFile file; if (fileGetFromVar(&file, parent)) { - if(file.data.mode == FM_READ || file.data.mode == FM_WRITE || file.data.mode == FM_READ_WRITE) { + if(file.data->mode == FM_READ || file.data->mode == FM_WRITE || file.data->mode == FM_READ_WRITE) { #ifndef LINUX - res = (FRESULT)f_lseek(&file.data.handle, (DWORD)(is_skip ? f_tell(&file.data.handle) : 0) + (DWORD)nBytes); + res = (FRESULT)f_lseek(&file.data->handle, (DWORD)(is_skip ? f_tell(&file.data->handle) : 0) + (DWORD)nBytes); #else - fseek(file.data.handle, nBytes, is_skip ? SEEK_CUR : SEEK_SET); + fseek(file.data->handle, nBytes, is_skip ? SEEK_CUR : SEEK_SET); #endif - fileSetVar(&file); } } } diff --git a/libs/filesystem/jswrap_file.h b/libs/filesystem/jswrap_file.h index 20634ac10..c2ce702a4 100755 --- a/libs/filesystem/jswrap_file.h +++ b/libs/filesystem/jswrap_file.h @@ -58,9 +58,8 @@ typedef struct { } PACKED_FLAGS JsFileData; typedef struct JsFile { - JsVar* fileVar; // this won't be locked again - we just know that it is already locked by something else - JsFileData data; - unsigned char _blank; //< this is needed as jsvGetString for 'data' wants to add a trailing zero + JsVar* fileVar; //< this won't be locked again - we just know that it is already locked by something else + JsFileData *data; } PACKED_FLAGS JsFile; // Called when stopping, to make sure all files are closed