fix: use pdf filepath to cache asset index

[atahanacar]

Don't compare pdf files byte by byte when adding it to OrderedAssetsCache. This causes performance issues as this operation is done for every page, which gets very expensive with many pages and big pdf sizes.

Cache the index of the asset using the pdf's file path.

Closes #1189

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 14 lines in your changes missing coverage. Please review.

Project coverage is 47.74%. Comparing base (c1c3059) to head (094a488).

Files with missing lines	Patch %	Lines
lib/components/canvas/image/pdf_editor_image.dart	0.00%	10 Missing ⚠️
lib/components/canvas/_asset_cache.dart	0.00%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1191      +/-   ##
==========================================
- Coverage   47.80%   47.74%   -0.06%     
==========================================
  Files         115      115              
  Lines        9094     9104      +10     
==========================================
  Hits         4347     4347              
- Misses       4747     4757      +10     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[QubaB]

It will be nice to push this fix as soon as possible because the long saving of notes with large pdf files is annoying (#1189).

[atahanacar]

I think this should be just a temporary fix as pdf file assets are still overwritten on every save. I will work on a better fix when I have the time, as I think there are more important bugs to fix.

[QubaB]

I think so.

I suppose that @adil192 did not expect such great use of large pdfs as I observe now. It seems to me that a lot of users do not use Saber to handwriting of notes but to comment pdfs.

[QubaB]

Lines 102-115 in lib/components/canvas/_asset_cache.dart

if (index == -1 && value is List<int>) {
      // Lists need to be compared per item
      final listEq = const ListEquality().equals;
      for (int i = 0; i < _cache.length; i++) {
        final cacheItem = _cache[i];
        if (cacheItem is! List<int>) continue;
        if (!listEq(value, cacheItem)) continue;
        index = i;
        break;
      }
    }
    log.fine('OrderedAssetCache.add: index = $index, value = $value');

should be removed. They are causing slow saving of pdfs by byte to byte comparison.

AddFileIndex and getFileIndex solve this and it is no more needed.

[atahanacar]

I think they should stay for non-file assets (don't know if they exist, will check when I get back home). We can have another method in OrderedAssetCache that accepts a file as an argument, so it uses the new filepath cache implemented in this PR. This way we can also avoid overwriting the entire asset file on every save altogether.

[QubaB]

This code was added by me and will be executed only when value is List it means file. It was introduced because _cache.indexOf(value); sometimes returned -1 index even if value was already in assets (it is probably problem of calculation of hash inside Object class.)

Imagine situation of two images (pdf files) in a note. When index == -1 (it means that asset is not in cache yet) image bytes will be compared to all already stored assets byte by byte which is slow. But your fix does not allow to call assets.add when it already knows ID of asset.

So in case of more images each new image will be at least once compared to all previously stored images.

Please test your code with more than one pdf files imported.

I hope my explanation is clear.

[atahanacar]

Yes, it is very clear. Thanks. I will do more testing and see how I can safely remove your code without causing the same issue you fixed. However, in the meantime I think this PR should be merged as soon as possible.

Also as a funny side note, I've been wanting to begin contributing code to this project for a long time but was procrastinating. The long saving bug your fix introduced actually made me finally start it, so I want to thank you for that 🤣

[QubaB]

The long saving bug your fix introduced actually made me finally start it, so I want to thank you for that 🤣

I am glad you started contributing. I am a FORTRAN programmer of scientific calculations and my contributions are clumsy. Someone else will do it better.

[atahanacar]

@adil192 please merge this as soon as possible, this is a critical issue that affects all platforms. I can't use the official or F-Droid builds because of this.

[Luciogi]

@adil192 any thoughts on this ?

[QubaB]

Please @adil192 consider to merge this branch. It improves time of saving note when it contains more large pdf files (avoids comparing each pdf file byte by byte).