Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Freezing in folders with too many files #738

Closed
EfronC opened this issue Oct 4, 2018 · 10 comments
Closed

Freezing in folders with too many files #738

EfronC opened this issue Oct 4, 2018 · 10 comments

Comments

@EfronC
Copy link

EfronC commented Oct 4, 2018

While testing the app, i found that on folder with thousands of images, the app freeze the computer, and is necessary to do an emergency restart.

Since i'm working on a contribution for the app(Support for XMP sidecar files), i investigated a little, and found that the problem is that you are using native fs.readdir(Inherited to fs-extra) to read the directory(checked on electron-io.js), and while this is not a problem with directories with a few files, in directories with thousands of files is a problem since this command loads all files to memory, and block the whole system(I think that the exact number of files to cause this varies depending on the requirements of your computer) Issue reference

While searching a solution, i found the readdir-enhanced package, which has an implementation to read a directory by streaming, something like this:

var readdir = require('readdir-enhanced');
var through2 = require('through2');
// EventEmitter API
readdir.stream('my/directory')
  .on('data', function(path) { ... })
  .on('file', function(path) { ... })
  .on('directory', function(path) { ... })
  .on('symlink', function(path) { ... })
  .on('error', function(err) { ... });

This is a package that is not installed in the project, but in case you don't want to install anything new, there seems to be already installed packages with implementations readdirp, but i haven't tested them since i'm not able to import the event-stream package.

But the problem with these is that the function is asynchronous, which causes the view to load before the process has ended. I'm not an expert in NodeJS, and this probably has a very simple solution, but that's why i'm asking if you could change that way to read directories for an streamed one, so that big folders can be loaded without freezing the app.

Thanks for your attention, hope this can be solved.

@uggrock
Copy link
Member

uggrock commented Oct 5, 2018

Hello,
and thanks for the extensive description and the suggestion for improvement! Did you investigated the issue in the last beta release. There, the search and actually the whole app was rewritten more or less from scratch. In the beta version. we are creating an index off all files asynchronously on every location opening, and later the search is done over the index.

@EfronC
Copy link
Author

EfronC commented Oct 5, 2018

When you say the last beta release, you mean the one is up on the code? Checking the code, i see the last commit in develop branch is from July 15, so if you are saying that this was changed after that commit, then it's probably a matter of waiting until you upload the next commit to check if your changes solve the problem.

@uggrock
Copy link
Member

uggrock commented Oct 8, 2018

Yes, I am talking about the develop branch https://github.com/tagspaces/tagspaces/tree/develop , and no there were no significant commits after Jule 15. What I wanted to point out, is that the code in the master branch is very different from the develop, so you if you plan to make any changes please use develop.

@uggrock
Copy link
Member

uggrock commented Oct 8, 2018

Is the issue here reproducible on the develop branch?

@EfronC
Copy link
Author

EfronC commented Oct 8, 2018

Yes, i'm working on Develop branch, i'm following the instructions from the readme, and yes, it's reproducible from Develop, thing goes like this:

  • Open Tagspaces.
  • Navigate to the parent folder of the folder to open.
  • The folder i'll open has approx. 2500 images.
  • Double click to open from the program.
  • Lap freeze and dies, has to restart.

Just for the record, the lap i tested on specifications:

RAM: 8GB.
CPU: i5-4210u.
Storage: 976GB free.

If by any chance you still has doubts, or only want to, you can check the repo i forked link, all the changes i've made are on develop, and is the one i'm actually using to do tests.

@uggrock
Copy link
Member

uggrock commented Oct 9, 2018

Are these images located in one folder without subfolders ? If yes this could be the reason for the freeze. TagSpaces is trying to create thumbnails for all these images....
I personally use TagSpaces for the management of my photo library on my laptop, currently containing 50k photos and it is working without issues. My photos are sorted in subfolders, containging up to 400 shots.

@EfronC
Copy link
Author

EfronC commented Oct 9, 2018

It had one, tried taking it out, but problem kept, Are you telling you have 50k in one folder without sub folder or 50k between many 400 pics folders? Because first one will be my case. Problem seems to be, like i said, that the fs's readdir takes too much memory to load the directory, i see that, when i open the folder, while it seems to slow down, it doesn't freeze, and seems to be working, but if i try to change from desktop(In Ubuntu), or basically anything, is when it gets frozen, and also, after the restart, when i check the .ts folder, i see it started creating the thumbnails, but stopped at one point(Suppose that when i restarted), so, is to understand that the program was taking too much resources that the computer can't do anything else or it freezes.

@loretoparisi
Copy link

In my case I have one folder that I'm reading with fs.readdir and it have started freezing when in this folder the number of files where 10K up. The problem is that it freezes the Node.js event loop, so it's pretty dangerous if you are doing this server-side, it basically will hang any node http connections, and causes the server to hang!

@loretoparisi
Copy link

loretoparisi commented Nov 2, 2018

by the way @Mithgol proposed here nodejs/node-v0.x-archive#388
an interesting approach to prevent the I/O Loop to block like

var fs = require('fs');
var clog = console.log;
var dir2read = '.';

var AsyncArrayProcessor = function (inArray, inEntryProcessingFunction) {
   var elemNum = 0;
   var arrLen = inArray.length;
   var ArrayIterator = function(){
      inEntryProcessingFunction(inArray[elemNum]);
      elemNum++;
      if (elemNum < arrLen) process.nextTick(ArrayIterator);
   }
   if (elemNum < arrLen) process.nextTick(ArrayIterator);
}

fs.readdir(dir2read, function(err, flist){
   if (err) {
      clog('Error reading directory ' + dir2read);
      clog(err);
      return;
   }
   var ProcessDirectoryEntry = function(entry){
      // This may be as complex as you may fit in a single event loop
      clog('Processing a directory entry: ' + entry);
   }
   AsyncArrayProcessor(flist, ProcessDirectoryEntry);
   clog('.readdir() callback is finished, event loop continues...');
});

@uggrock
Copy link
Member

uggrock commented Jul 15, 2021

we introduced pagination -> closing

@uggrock uggrock closed this as completed Jul 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants