Slight simplification of chars().count()

[gilescope]

Slight simplification: No need to call len(), we can just count the number of non continuation bytes.

I can't see any reason not to do this, can you?

[rust-highfive]

r? @joshtriplett

(rust-highfive has picked a reviewer for you, use r? to override)

[gilescope]

Is there a github command to kick off a performance comparison? I'm not sure we'd see much of an improvement but it seems a simpler way to define the function.

[arniu]

How about self.iter.filter(|byte| !utf8_is_cont_byte(byte)).count()?

Should be self.iter.filter(|byte| !utf8_is_cont_byte(**byte)).count()

[SkiFire13]

It doesn't look like this makes any difference https://godbolt.org/z/vKhTo9
The only difference I can see is that there are two additional less movs and two additional less subs, so a tiny regression improvement (Edit: godbolt swapped the generated code in the diff).

Is there a github command to kick off a performance comparison?

There's one to profile the compiler, but I don't this is the case. Usually you just locally run the relevant benchmarks and post the results.

ps: Does anyone know why the related benches are in library/alloc/benches/str.rs?

[bugadani]

The only difference I can see is that there are two additional movs and two additional subs, so a tiny regression.

I think you're looking at the diff backwards, Editor #2 seems to be the smaller one. Not a terribly big difference, though. Either way the code requires a bit of a "what's going on here?" thinking, but I find the shorter version easier on my brain.

How about self.iter.filter(|byte| !utf8_is_cont_byte(byte)).count()?

Even better IMO.

[bluss]

Nice simplification!

[gilescope]

Godbolt has nice diffs! It shows that filter count has exactly the same instructions as map sum so I'll change it to filter count (as the intent is clearer).

[rust-log-analyzer]

The job x86_64-gnu-llvm-9 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

   Compiling unwind v0.0.0 (/checkout/library/unwind)
error[E0308]: mismatched types
  --> library/core/src/str/iter.rs:50:53
   |
50 |         self.iter.filter(|&byte| !utf8_is_cont_byte(byte)).count()
   |                                                     |
   |                                                     |
   |                                                     expected `u8`, found `&u8`
   |                                                     help: consider dereferencing the borrow: `*byte`
error: aborting due to previous error

For more information about this error, try `rustc --explain E0308`.
error: could not compile `core`

[gilescope]

(@SkiFire13 thanks for pointing out where the benchmarks were.)
Running ./x.py bench library/alloc --test-args chars_count

Before:
test str::chars_count::long_lorem_ipsum ... bench: 734 ns/iter (+/- 134)
test str::chars_count::short_ascii ... bench: 19 ns/iter (+/- 3)
test str::chars_count::short_mixed ... bench: 20 ns/iter (+/- 5)
test str::chars_count::short_pile_of_poo ... bench: 19 ns/iter (+/- 4)

After:
test str::chars_count::long_lorem_ipsum ... bench: 730 ns/iter (+/- 93)
test str::chars_count::short_ascii ... bench: 18 ns/iter (+/- 2)
test str::chars_count::short_mixed ... bench: 19 ns/iter (+/- 2)
test str::chars_count::short_pile_of_poo ... bench: 19 ns/iter (+/- 2)

So seems no noticable difference in performance which given it only shaves a few instructions off I guess is to be expected.

[joshtriplett]

Looks reasonable to me. Nice improvement.

@bors r+

[bors]

📌 Commit a623ea5 has been approved by joshtriplett

[klensy]

fn count(self) -> usize {
    self.iter.filter(|&&byte| !utf8_is_cont_byte(byte)).take(usize::MAX).count()
}

This somehow generates smaller code, but maybe it slower?

[SkiFire13]

That doesn't vectorize the loop so yes, it would probably be slower