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

Remove unessesary iteration #1

Merged
merged 1 commit into from
Mar 16, 2022
Merged

Conversation

MggMuggins
Copy link
Contributor

I was reading your code and noticed that you were iterating the string twice where you didn't need to. Iterator::count will consume the iterator. Of course, chars.clone() only duplicates the iterator (since the iter holds only a reference to the string), but it still needs to consume that copy to count the number of items. This extra iteration is unnecessary because the str (that you got from AsRef::as_ref) stores the length of its buffer.

On my hardware, your code gets 2227 ns average. With my changes, I got 1540 ns. The kicker is cargo test --release though... 90 ns with yours and 78 ns with mine. Looks like the compiler guys are better at optimizing than either of us.

There's a couple other versions of basically the same algorithm that are more aggressively optimized using arrays, the luhn3 and isin crates are the ones I'm thinking of.

See !1 for details
@grantshandy
Copy link
Owner

Thanks! I didn't think about using str::len too, this looks great.

@grantshandy grantshandy merged commit c11f1ca into grantshandy:main Mar 16, 2022
@pacak
Copy link

pacak commented May 31, 2023

the luhn3 and isin crates are the ones I'm thinking of.

FWIW luhn3 uses SWAR plus arrays to calculate/validate decimal Luhn checksum which lowers the speed benchmark from this

The validation took an average of 54 nanoseconds (0 miliseconds)
Validating 10,000,000 imei codes took 1000 miliseconds (1 seconds)

to this

The validation took an average of 39 nanoseconds (0 miliseconds)
Validating 10,000,000 imei codes took 853 miliseconds (0 seconds)
diff --git a/tests/speed.rs b/tests/speed.rs
index 663c7b6..256eece 100644
--- a/tests/speed.rs
+++ b/tests/speed.rs
@@ -24,7 +24,7 @@ fn speed() {
 fn test() -> i64 {
     let time_before = chrono::Utc::now();
 
-    assert!(imei::valid("490154203237518"));
+    assert!(luhn3::decimal::valid("490154203237518".as_bytes()));
 
     // pass "--nocapture" to show time difference
     let difference = chrono::Utc::now()

Now, the benchmark itself comes with some overhead - if I comment out the check and simply run the measuring loop result is

The validation took an average of 31 nanoseconds (0 miliseconds)
Validating 10,000,000 imei codes took 775 miliseconds (0 seconds)

Which means imei code takes 54-31 = 23ns and luhn3 takes 39-31 = 8ns.

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