refactor: Make it thread-safe #54
Conversation
|
Hello, I'm going to admit my lack of familiarity with thread-safety in rust. Does the entire library need to be fully thread-safe in order to be used in a multi-threaded context? I understand why you might want the things passed in and out of the library to be fully thread-safe, but I'm unclear on if every internal bit has to be Arc. I'm curious about the context where this doesn't work. |
|
Actually now that I have read a bit more and tested it I see the point. I'll review this later this week. Thanks |
|
Correct. The library must be thread-safe to be used in a multi-threaded context, otherwise in Rust, it won't even compile. I recommend using Arc instead of Rc as it provides library users with more flexibility. I also really need it in my application. I've already forked the repo and used it. |
|
I'm about half done with the review. Do you have benchmarks for the before-after change (I'd like to include them in the patch notes). |
Beforeaztec time: [51.649 µs 51.689 µs 51.737 µs] codabar time: [1.7609 µs 1.7645 µs 1.7682 µs] code39 time: [3.6943 µs 3.7049 µs 3.7145 µs] code93 time: [1.0691 µs 1.0760 µs 1.0820 µs] datamatrix time: [13.575 µs 13.594 µs 13.613 µs] ean8 time: [1.1383 µs 1.1400 µs 1.1418 µs] ean13 time: [3.9190 µs 3.9289 µs 3.9390 µs] itf time: [1.4751 µs 1.4870 µs 1.5113 µs] maxicode time: [70.059 µs 70.152 µs 70.264 µs] pdf417 time: [96.889 µs 97.199 µs 97.718 µs] qrcode time: [386.03 µs 387.06 µs 388.17 µs] rss14 time: [8.6547 µs 8.6795 µs 8.7127 µs] rss_expanded time: [6.7197 µs 6.7342 µs 6.7577 µs] telepen time: [2.0463 µs 2.0526 µs 2.0593 µs] upca time: [26.738 µs 26.782 µs 26.827 µs] upce time: [945.95 ns 947.34 ns 948.71 ns] multi_barcode time: [2.7054 ms 2.7094 ms 2.7135 ms] Afteraztec time: [51.292 µs 51.378 µs 51.468 µs] codabar time: [1.7456 µs 1.7565 µs 1.7731 µs] code39 time: [3.6865 µs 3.6921 µs 3.6981 µs] code93 time: [1.0543 µs 1.0574 µs 1.0612 µs] datamatrix time: [13.387 µs 13.420 µs 13.454 µs] ean8 time: [1.1182 µs 1.1206 µs 1.1234 µs] ean13 time: [3.8146 µs 3.8209 µs 3.8275 µs] itf time: [1.4424 µs 1.4456 µs 1.4492 µs] maxicode time: [68.368 µs 68.514 µs 68.675 µs] pdf417 time: [94.042 µs 94.490 µs 95.307 µs] qrcode time: [350.42 µs 352.26 µs 354.29 µs] rss14 time: [8.4125 µs 8.4346 µs 8.4562 µs] rss_expanded time: [6.5365 µs 6.5546 µs 6.5740 µs] telepen time: [1.9871 µs 1.9932 µs 1.9999 µs] upca time: [26.086 µs 26.204 µs 26.390 µs] upce time: [925.68 ns 928.49 ns 931.35 ns] multi_barcode time: [2.6075 ms 2.6118 ms 2.6169 ms] |
|
Thank you, I'll finish review and have it merged tomorrow. Due to the changes to the public surface this is going to be v0.6.0 |
|
I'm going to spend time today trying to clean up uses or Arc, I did switch the definition of the result point callback to require the function to be Send+Sync so that it can be used in static contexts. |
The current implementation of rxing uses Rc for reference counting, which is not thread-safe. This limitation restricts the library’s use in multi-threaded applications where safe concurrent access to shared resources is required.
Passed all test cases successfully.