search

awslabs / coldsnap

A command line interface for Amazon EBS snapshots
Apache License 2.0
178 stars 24 forks source link

Fine-tune the Ordering for changed_blocks_count #309

Closed wang384670111 closed 5 months ago

wang384670111 commented 6 months ago

https://github.com/awslabs/coldsnap/blob/a60762c53f5ba3e8a0b6fa3d972ec082739f33df/src/upload.rs#L221 https://github.com/awslabs/coldsnap/blob/a60762c53f5ba3e8a0b6fa3d972ec082739f33df/src/upload.rs#L400 Here changed_blocks_count is employed solely for counting purposes, rather than for synchronizing access to other shared variables. While SeqCst guarantees program correctness, it can also impact performance. Therefore, using Relaxed is sufficient to maintain the program's correctness without adversely affecting its efficiency.

wang384670111 commented 6 months ago

Thanks for your contribution! I agree that Relaxed ordering is still correct here since we don't synchronize access to other data based on the counter.

As separate feedback, do you mind making modifying the commit message? Something more specific like this would be great:

Use relaxed ordering for atomic block change counter

I've made the necessary changes to the commit information. May I inquire about the possibility of merging this pull request at your earliest convenience?