Skip to content

Conversation

riccio8
Copy link
Contributor

@riccio8 riccio8 commented Sep 14, 2025

Related to #307 generate_thumbnails

  • Added the use of mimalloc (Microsoft allocator) as the global allocator
  • Use of rayon ThreadPoolBuilder instead of std Thread

This is a first commit, I plan to bring many more optimizations

@CyberTimon
Copy link
Owner

Thank you, this looks great! I’m planning to release the next version later today. Do you want me to go ahead and merge this now, or are you planning to add more optimizations to the PR?

@riccio8
Copy link
Contributor Author

riccio8 commented Sep 14, 2025

Thank you, this looks great! I’m planning to release the next version later today. Do you want me to go ahead and merge this now, or are you planning to add more optimizations to the PR?

If u want u can merge this in the next release, while I add more optimizations.

Thx a lot

@CyberTimon
Copy link
Owner

I just tested your PR, and from what I can tell, it generates all the thumbnails in the background and only displays them once they’re all ready. In my experience, this approach introduces more lag compared to progressively generating and showing thumbnails as they become available. Am I missing something?

@riccio8
Copy link
Contributor Author

riccio8 commented Sep 14, 2025

I just tested your PR, and from what I can tell, it generates all the thumbnails in the background and only displays them once they’re all ready. In my experience, this approach introduces more lag compared to progressively generating and showing thumbnails as they become available. Am I missing something?

In reality, the logic remains the same; the only thing I changed is the type of thread pool. Instead of using the one from the std lib, I use the one from rayon, and I configure it to be used without saturating all the cores (which slows down tauri rendering). I also decided to use mimalloc, the memory allocator developed by Microsoft as a global allocator, which is much more efficient than the system ones and, above all, cross-platform.

So basically, the logic remains the same, but I don't saturate all the CPUs, which would block rendering until they finish, and I use an allocator optimized for storing large amounts of heavy data in memory.

It's not a game changer; in fact, I plan to make other changes, but it's a start.
Did it seem better or worse than the version you are currently using?

Thx a lot for ur feedback

@CyberTimon
Copy link
Owner

Hey!

I noticed that while the core logic wasn’t changed, with your PR the thumbnails only appear once all of them finish loading, which blocks the UI until everything is ready.

On the main branch (without your PR), the thumbnails load progressively - they appear smoothly as they’re generated - and the UI stays responsive.

I can take another look in the next few days, but for now it feels like the current PR behavior is actually a step back.

Thanks!

@riccio8
Copy link
Contributor Author

riccio8 commented Sep 14, 2025

Hey!

I noticed that while the core logic wasn’t changed, with your PR the thumbnails only appear once all of them finish loading, which blocks the UI until everything is ready.

On the main branch (without your PR), the thumbnails load progressively - they appear smoothly as they’re generated - and the UI stays responsive.

I can take another look in the next few days, but for now it feels like the current PR behavior is actually a step back.

Thanks!

That's right, I took a closer look at rayon's install function and it's a blocking function, so while thread progressively passed the thumbnails created to the UI, poll.install waits for the generation to finish, which is slower.

To combine the benefits, I thought we could merge the two logics, using poll.install inside a separate thread. It's a small change, but it should make loading smoother.

Obviously, I'll work on other possible optimizations, and I hope to be able to merge them into the release after the next one.

Let me know what you think. Would that be okay?

@riccio8
Copy link
Contributor Author

riccio8 commented Sep 14, 2025

Regarding the installation documentation, you can find more information here:
rayon install

@CyberTimon
Copy link
Owner

I now have the problem that it emits the same file many times and displays the thumbnails duplicated:

This is your PR:
image

And this is main:
image

Do you have a idea why this might happen?

@CyberTimon
Copy link
Owner

I try to fix it - give me some minutes.

@CyberTimon
Copy link
Owner

Related: #317

@CyberTimon
Copy link
Owner

I couldn't find a fix yet. Will take a look again in the next days - feel free to try resolve the issue.

@riccio8
Copy link
Contributor Author

riccio8 commented Sep 14, 2025

I couldn't find a fix yet. Will take a look again in the next days - feel free to try resolve the issue.

Sure, I'll work on it in the next few days. It's still unstable, so I recommend not merging with the next release. Maybe I have a better option, a static thread, and then call its .spawn function, which isn't blocking, unlike .install.

I noticed that the proposed change (os threads and rayon pool) would also have the same problem, so I'll try to fix that.

About the file duplication issue it's really weird, I saw that you tried to add a hash for the photos so you know which thumbnails were generated, very strange as a behavior

@riccio8
Copy link
Contributor Author

riccio8 commented Sep 14, 2025

Which OS gives this problem?

@CyberTimon
Copy link
Owner

I tried it on Windows 11 :)

@riccio8
Copy link
Contributor Author

riccio8 commented Sep 15, 2025

Hey @CyberTimon
I made a few changes:

  • I replaced the install function with spawn, which is non-blocking
  • After reading some issues where sudden crashes occur (such as issues #322), I implemented an auto-log system which generates a log on stderr and in a file called RapidRaw.log every time there is a panic or undefined behavior

Let me know what you think, thanks :)

U-DELL-RICKY\riccio added 2 commits September 15, 2025 21:13
This reverts commit b436903.
This reverts commit 2ae86a4.

 On branch main
 Your branch is ahead of 'origin/main' by 1 commit.
   (use "git push" to publish your local commits)

 Changes to be committed:
	modified:   src-tauri/Cargo.lock
	modified:   src-tauri/Cargo.toml

 Changes not staged for commit:
	modified:   src-tauri/src/main.rs
@riccio8 riccio8 reopened this Sep 15, 2025
@riccio8
Copy link
Contributor Author

riccio8 commented Sep 15, 2025

@CyberTimon done, I tested it

@aNNiMON
Copy link

aNNiMON commented Sep 15, 2025

@riccio8

  • I implemented an auto-log system

I've tried freshly built artifact and upon crashing there is nothing in the logs. RapidRaw.log literally 0 bytes

@riccio8
Copy link
Contributor Author

riccio8 commented Sep 16, 2025

@riccio8

  • I implemented an auto-log system

I've tried freshly built artifact and upon crashing there is nothing in the logs. RapidRaw.log literally 0 bytes

I set only error as filter, I'll change it into Debug or Info for capturing more details

@aNNiMON
Copy link

aNNiMON commented Sep 16, 2025

@riccio8 what about setting logging level based on the env variable RUST_LOG with the error level as a default? So that I can set RUST_LOG=debug and see detailed results.

@CyberTimon
Copy link
Owner

@riccio8 Just tested the PR and everything is like 500x times slower? Is this because of the debugging? Because I see a lot of debugging in the console, but the application itself (thumbnail loading, image editing etc., is like 500 times slower)

2025-09-16.11-31-24.mp4

@riccio8
Copy link
Contributor Author

riccio8 commented Sep 16, 2025

@riccio8 Just tested the PR and everything is like 500x times slower? Is this because of the debugging? Because I see a lot of debugging in the console, but the application itself (thumbnail loading, image editing etc., is like 500 times slower)

2025-09-16.11-31-24.mp4

Yeah, that's because of the Debug filter settled as level of log. I'll change following @aNNiMON 's suggestions, Error should be a lot more fluid as experience.

This afternoon I'll edit and text.

By the way, do you think a directory with tests would be useful?

@CyberTimon @aNNiMON

 interactive rebase in progress; onto edcbde1
 Last command done (1 command done):
    pick c60cd73 added a more rapid logger setted using RUST_LOG env var
 No commands remaining.
 You are currently rebasing branch 'main' on 'edcbde1'.

 Changes to be committed:
	modified:   src-tauri/Cargo.toml
	modified:   src-tauri/src/main.rs
@riccio8
Copy link
Contributor Author

riccio8 commented Sep 16, 2025

Done, is it better?
@CyberTimon @aNNiMON

@CyberTimon
Copy link
Owner

Will test it out in the coming days. Sorry I haven't had much time lately. Thank you :)

@CyberTimon CyberTimon self-requested a review as a code owner September 22, 2025 18:01
@riccio8
Copy link
Contributor Author

riccio8 commented Sep 22, 2025

Will test it out in the coming days. Sorry I haven't had much time lately. Thank you :)

Np, let me know

@riccio8
Copy link
Contributor Author

riccio8 commented Sep 22, 2025

Hey @CyberTimon @aNNiMON
I was thinking that mapping files in memory wouldn't be a bad idea, as you map 1000 at a time, and then pass the mapping reference to progressively generate thumbnails. You can do this using the crate https://docs.rs/memmap2/latest/memmap2/.

What do you think?

@CyberTimon
Copy link
Owner

That sounds interesting! My knowledge of mappings is somewhat limited, but feel free to give it a try 💯. In the meantime, I’ll go ahead and merge this, since the logging and related improvements are definitely things I’d like included in the next release.

@CyberTimon CyberTimon merged commit b50647f into CyberTimon:main Sep 22, 2025
9 checks passed
@riccio8
Copy link
Contributor Author

riccio8 commented Sep 23, 2025

That sounds interesting! My knowledge of mappings is somewhat limited, but feel free to give it a try 💯. In the meantime, I’ll go ahead and merge this, since the logging and related improvements are definitely things I’d like included in the next release.

I'll let you know. I don't have much time these days, but I'll work on it. Would you prefer me to open a pull request or an issue and work on it?

@riccio8
Copy link
Contributor Author

riccio8 commented Sep 23, 2025

@CyberTimon

Sorry for mentioning, but I implemented it, tested it, and it seems to perform better. I tried it on a 4TB NAS in RAID and loaded a thousand images. It took a little while at first, 2-3-4 seconds, and then loaded them all.

How do u want me to pull the request? thx

@CyberTimon
Copy link
Owner

Yes that would be great! Thank you for all the work :)

@riccio8
Copy link
Contributor Author

riccio8 commented Sep 24, 2025

here it is: #347 (comment)
@CyberTimon

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

Successfully merging this pull request may close these issues.

3 participants