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

Document whether RollingHash is thread-safe #10

Closed
Rabadash8820 opened this issue Jun 10, 2024 · 4 comments
Closed

Document whether RollingHash is thread-safe #10

Rabadash8820 opened this issue Jun 10, 2024 · 4 comments

Comments

@Rabadash8820
Copy link

Hello and thanks for maintaining this project! The API docs for the rollingHash param of the VcEncoder.Encode method state:

Manually provide a VCDiff.Encoders.RollingHash instance that can be reused for multiple encoding instances of the same block size. If you provide a VCDiff.Encoders.RollingHash instance, you must dispose of it yourself

However, I can't tell for sure whether RollingHash can be used by multiple encoding instances that are running in parallel. Is RollingHash thread-safe? It would be great to document this either in the API docs or somewhere in this repo.

@chyyran chyyran pinned this issue Jun 11, 2024
@chyyran
Copy link
Member

chyyran commented Jun 11, 2024

RollingHash is never mutated once created so it should be safe to use in parallel.

@chyyran chyyran closed this as completed Jun 11, 2024
@Rabadash8820
Copy link
Author

Rabadash8820 commented Jun 11, 2024

Sounds good; thanks for getting back to me. The main field I was concerned about is v_shuf. I see it referenced on this line (shown below) and wasn't sure if that mutates v_shuf (I don't know much about AVX2).

c_v = Avx2.PermuteVar8x32(c_v, v_shuf);

Also, to be clear, this Issue was about documenting whether RollingHash is thread-safe, as I'm sure other users of this library might wonder if it can be used in a multi-threaded environment. Pinning this Issue is a start, but it would be good to have this info in the Readme.

@chyyran
Copy link
Member

chyyran commented Jun 11, 2024

No worries, I'm a bit backed up on things at the moment so I'll be updating the documentation when I get the chance.

For reference, Avx2.PermuteVar8x32 is a shim for _mm256_permutevar8x32_epi32, which doesn't mutate its arguments in place.

@Rabadash8820
Copy link
Author

Sounds good, thanks for your help!

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

2 participants