aadsm / jsmediatags

Media Tags Reader (ID3, MP4, FLAC)
Other
745 stars 128 forks source link

Bad performance in for loop? #111

Closed CodeF0x closed 5 years ago

CodeF0x commented 5 years ago

Hi!

I'm reading meta information of n files. The amount of those files varies gretly. I'm doing that in a for loop and this process blocks my entire application for ~ 1 to 2 seconds, depending on how many file there are.

I guess it's this library that causes this issue because when using dummy values instead of values gotten from the files, that heavy delay does not appear or is at least way smaller. (Using files without any meta information are also not affected of the delay).

I've implemented it like this:

listMusicFiles: function(files) {
    // Not important stuff here

    for (let i = 0; i < files.length; i++) {
      const container = document.createElement('div');
      container.setAttribute('data-file-path', files[i].path);
      container.addEventListener('click', function() {
        play(this.getAttribute('data-file-path'));
      });

      const name = document.createElement('div');
      const album = document.createElement('div');
      const artist = document.createElement('div');

      new media.Reader(files[i].path)
        .setTagsToRead(['title', 'album', 'artist'])
        .read({
          onSuccess: tag => {
            let songNameSplitted = files[i].path.split('/');
            songNameSplitted = songNameSplitted[songNameSplitted.length - 1];

            name.innerText = tag.tags.title ? tag.tags.title : songNameSplitted;
            album.innerText = tag.tags.album ? tag.tags.album : 'Unknown';
            artist.innerText = tag.tags.artist ? tag.tags.artist : 'Unknown';
          },
          onError: err => console.error(err)
        });

      container.appendChild(name);
      container.appendChild(album);
      container.appendChild(artist);
      list.appendChild(container);
    }
  }

Is there something I can do about this? Or is it the fact that I create that many DOM elements at once?

Thanks, Tobi "CodeF0x"

PS. Thanks for this great library! It makes things so much easier!

aadsm commented 5 years ago

Hard to know without tracing it. Loading the files is done in async way so that wouldn’t block the browser. Reading the metadata is in a tight loop but it’s usually very fast since there are not that many tags to read. Initially I thought it could be reading the cover image but that’s not even a tag you are requesting.

However, I do recommend splitting this function into 2. One that only gathers the data and the other that generates the UI from that data. You are making multiple DOM updates that should be batched. Try to use a DocumentFragment instead of updating the DOM directly and then add that fragment to the DOM at the end.

CodeF0x commented 5 years ago

Thanks for your fast reply and your help. I really appreciate it!

I tried as you suggested with DocumentFragment, but that unfortunately helped just a little. I maybe should clarify my problem. The process is not blocked in a way that nothing happens and suddenly there's a list of 300 elements. It's more "blocked" in a way that you can see the creation of the elements "lagging". I'll attach a GIF for better understanding.

lagging

CodeF0x commented 5 years ago

After almost two months of some testing and researching, I came to the conclusion that it must be a problem with Electron, and not with this library, as I couldn't replicate the issue in any browser on any operating system.

Thanks anyway for your help!

aadsm commented 5 years ago

Oh interesting, I was actually not expecting this. Thank you for the update.