Accelerate path.listdir()

[Staars]

Description:

Related to #18906

@btsimonh I did not figure out the meaning of

                                        if (strcmp(fn, ".") && strcmp(fn, "..")) {
                                            be_pushstring(vm, fn);
                                            be_data_push(vm, -2);
                                            be_pop(vm, 1);
                                        }

Maybe the ìf is not needed with the newer API, maybe I have overlooked something.

Speed increase was by a factor of 7 for 200 files. List output in Berry was truncated and is another problem.

Checklist:

• The pull request is done against the latest development branch
• Only relevant files were touched
• Only one feature/fix was added per PR and the code change compiles without warnings
• The code change is tested and works with Tasmota core ESP8266 V.2.7.4.9
• The code change is tested and works with Tasmota core ESP32 V.2.0.10
• I accept the CLA.

NOTE: The code change must pass CI tests. Your PR cannot be merged unless tests pass

[btsimonh]

It's checking for and ignoring '.' and '..' - of course they may not appear in some filesystems?
That part came directly for the original code.....
Thanks for attempting reproduction, and nice to see the speed improvement :).
I'll try it out later...

[Staars]

Ah, I remember that issues with . and ..

@s-hadinger I would prefer, that Berry does not hide hidden entries, to better monitor file system pollution. But maybe there are more reasons to keep this check. I guess it was introduced for the web UI, wasn‘t it?

[btsimonh]

but... did you try '/somefolder/../somefolder' - maybe there is no support?
I'm worried about my zap(folder) function - if the FS does support the above!!!
But in general, it should be the berry app which deals with the raw returned data.

[Staars]

I am not sure, that I did understand this correctly.
Did you mean the general functionality of subfolders with this PR?
This seems to work, as I did another run with:
200 files in "/"
and
9993 files in "/torture"

The result was correct again and indeed performance in the root folder was better now with less than 10000 files.
Or wasn't that your question?

[btsimonh]

no, my question was specifically about the . and ..
i.e., if someone listed '/torture/../torture', do they get the same result as '/torture'? If not, then providing '..' is only an indication of the fact we're in a subfolder. To me, '.' has little use? And '..' is dangerous (think of my ZAP() function - it could (would currently) recurse into the root folder through finding '..' - it currently refuses to zap('/') or zap('')).

[btsimonh]

observations of TAS ref large folders: (test folder is a real folder on SD card with 2238 files).

1/ listing through the GUI. Whilst listing a (large) folder in the GUI, everything else is frozen. It stopped at ~1980 files. Tas is now unresponsive... (took two power offs to get back - got my steps in today).
(note: there is a non-file-closure bug in the GU llisting - but only for hidden files, so not the cause of the hang).

2/ listing via berry with this PR:

import path
print('starting')
var t = path.listdir('/sd/timelapse/test5')
print(size(t))

Result in about 1s:

starting
2238

Use cases:

Viewing an existing SD card.

why? - I think we can discount existing information on an SD as 'extraneous'.

stuff we wrote from TAS

with esp32cam, we can do motion capture and timelapse.

In both applications, we're in control of how much we write. i.e. we can start a new folder when we hit a certain quantity.
What quantity would we like to capture?

timelpase

Well for timelapse, the min cron interval is 1 minute (we can do shorter with timer) - so 1440 per day (24 hours). This seems a reasonable number to support in a folder. However, each frame (in my current implementation) is two files - the jpeg and some metadata (time, etc.). So 2880 per day. Still not unreasonable from the above....

motion

a motion event per minute seems unreasonbable. So above applies. If we keep < 3000, we're in good shape.

For me, this PR is a big improvement on the existing listdir function.

Final note:

In the above code, if I print(t), it goes away for what seems forever (yes, I gave up)
if I print(t[0]), I see the first REAL file (index.html) (not '.' or '..') - from SD card.

thanks again @Staars for a sound solution.

[btsimonh]

haha - what would be really cool is if path.listdir('somename.zip#') worked with your implementation - it can't at the moment because zipFS rejects that.... but maybe the code would support it with a minor mod?

[btsimonh]

something i did not notice last night.
when I printed the last file, the string is the full path /sd/timelapse/test5/index.html

This does not match the previous implementation where the filenames supplied were just the names. It would use less ram if only the names were returned?

[Staars]

That's basically the default behavior of the Arduino lib, which I would expect to be the standard in the Berry implementation too. But as always ... personal preference.

As working with large file numbers will not be possible (aka is practically impossible) without a PSRAM board, we "only waste" PSRAM here. Unless you run out of this, there is no real world problem.

My suggestion is, to leave the implementation according to the Arduino standard behavior for this PR.
If you later really run in to trouble with this, then a change should be no problem. I assume, that currently there are not hordes of Berry coders out there, that deal with >10000 files in Tasmota, so a later small API change should be feasible .

But I would gladly leave that decision to @s-hadinger .

[btsimonh]

I'm happy either way. I just wonder how many people already have path.listdir in use. Indeed, I think it's used during boot.... so the PR may actually already be breaking searching for certain berry files. @s-hadinger will know :).

[Staars]

Okay, for testing ... the old behavior:
(starting at line 148)

                        if (dir) {
                            String fpath;
                            String fname;
                            switch (action){
                                case MPATH_LISTDIR:
                                    dir->seekDir(0);
                                    fpath = dir->getNextFileName();
                                    while (fpath.length() != 0) {
                                        fname = fpath.substring(fpath.lastIndexOf("/") + 1);
                                        const char * fn = fname.c_str();
                                        be_pushstring(vm, fn);
                                        be_data_push(vm, -2);
                                        be_pop(vm, 1);
                                        fpath = dir->getNextFileName();
                                    }
                                    break;

Tested the speed with this underengineerd benchmark:

print(tasmota.memory())
import path
var m = tasmota.millis()
var r = path.listdir("/")
print(size(r))
var t = path.listdir("/torture")
print(size(t))
var d = tasmota.millis() - m
print("duration in millis:", d)
print(tasmota.memory())

giving this result:

12:41:46.147 {'stack_low': 3.71094, 'program_free': 1077, 'program': 1802, 'psram': 4093, 'stack_size': 8, 'flash_real': 16384, 'iram_free': 5, 'flash': 16384, 'frag': 8, 'heap_free': 116, 'psram_free': 4081}
12:41:46.335 104
12:42:15.437 9993
12:42:15.438 duration in millis: 29281
12:42:15.447 {'stack_low': 3.71094, 'program_free': 1077, 'program': 1802, 'psram': 4082, 'stack_size': 8, 'flash_real': 16384, 'iram_free': 5, 'flash': 16384, 'frag': 7, 'heap_free': 115, 'psram_free': 3611}
12:42:32.852 {'stack_low': 3.71094, 'program_free': 1077, 'program': 1802, 'psram': 4082, 'stack_size': 8, 'flash_real': 16384, 'iram_free': 5, 'flash': 16384, 'frag': 7, 'heap_free': 115, 'psram_free': 3603}
12:42:33.038 104
12:43:01.502 9993
12:43:01.503 duration in millis: 28641
12:43:01.512 {'stack_low': 3.71094, 'program_free': 1077, 'program': 1802, 'psram': 4081, 'stack_size': 8, 'flash_real': 16384, 'iram_free': 5, 'flash': 16384, 'frag': 7, 'heap_free': 114, 'psram_free': 3468}

(just two runs of the same, seems to cache a bit)

Speed seems to be the same (yes, 9993 files is insanely slow).

[btsimonh]

just for the record, I tested before and after the above.

using the 'filenames only' code above shaved a few ms off my ~1s for 2238 files on SD card.
I'm happy that with the above change, this PR performs the same as the old path.listdir, but somewhat more efficiently.
I also checked first and last filenames, and all the data appears to be there :).

before:

{'stack_low': 3.70703, 'program_free': 1984, 'program': 1447, 'psram': 4092, 'stack_size': 8, 'flash_real': 4096, 'iram_free': 39, 'flash': 4096, 'frag': 4, 'heap_free': 78, 'psram_free': 2607}
2238
duration in millis: 1161
2238
duration in millis: 939
{'stack_low': 3.70703, 'program_free': 1984, 'program': 1447, 'psram': 4057, 'stack_size': 8, 'flash_real': 4096, 'iram_free': 39, 'flash': 4096, 'frag': 4, 'heap_free': 78, 'psram_free': 2409}

after:

{'stack_low': 3.73828, 'program_free': 1984, 'program': 1447, 'psram': 4093, 'stack_size': 8, 'flash_real': 4096, 'iram_free': 39, 'flash': 4096, 'frag': 4, 'heap_free': 78, 'psram_free': 3301}
2238
duration in millis: 1127
2238
duration in millis: 931
{'stack_low': 3.73828, 'program_free': 1984, 'program': 1447, 'psram': 4090, 'stack_size': 8, 'flash_real': 4096, 'iram_free': 39, 'flash': 4096, 'frag': 4, 'heap_free': 78, 'psram_free': 3161}

[Staars]

Thanks for testing.
Meanwhile I also think, that we should stick to the old behavior. Changes pushed.
Now the only functional change would be to not filter out . and .., which IMHO makes sense for a function, that usually works behind the scenes and not directly in GUI.

[btsimonh]

on my SD, I do not see '.' or '..' anyway, unless they hide at file 1000!