vlib: refactor empty string checks

[ttytm]

Instead of checking empty strings via str_var.len > 0, str_var.len == 0, str_var.len < 1
the PR moves towards using str_var != '' and str_var == ''.

The former is longer to write, has no performance benefit, and, being less expressive, it comes with more mental overhead and requires more context to comprehend what (type) the condition checks. So imho making it unnecessary hard.

If a PR for v vet has a chance, I'd give it a shot implementing it. Imho it would be fitting to notify about this.

[JalonSolov]

The problem with this is that string comparisons are much slower than int comparisons. This check is very quick, but will still be slower with the change.

And lots of little slowdowns can add up to a large slowdown overall.

[ttytm]

I couldn't find the check for '' being slower.

[ttytm]

No doubt it makes sense when the string needs to be a certain length to check for then len. But for empty strings?

[JalonSolov]

As I said, it's a very small difference, but it is there. Just look at line 732 in builtin/string.v to see the code that's executed for == for strings.

First, it checks to see if the str field == 0. Then it compares the lengths of the strings, then it checks to see if the length is > 0. Then it calls vmemcmp. Worse, it will always call vmemcmp (since none of the other tests will fail), even for empty strings.

All that, to compare 2 empty strings.

Whereas a comparison of 2 ints is... simple and straight-forward. A single CPU instruction on most systems.

[ttytm]

As I said, it's a very small difference, but it is there. Just look at line 732 in builtin/string.v to see the code that's executed for == for strings.

As far as I can interpret it's made sure that the string len is compared when using the == on strings, so it should make no difference in performance.
From the check below it varies which on is faster and differences between runs are in a non significant range. It has to be measurable if there is a performance diff.

import time

str := ''
non_empty := 'This is my non empty string'

sw := time.new_stopwatch()

for _ in 0 .. 1_000_000 {
	if str.len == 0 {
		assert true
	}
	if non_empty.len != 0 {
		assert true
	}
}

dump(sw.elapsed())

sw2 := time.new_stopwatch()

for _ in 0 .. 1_000_000 {
	if str == '' {
		assert true
	}
	if non_empty != '' {
		assert true
	}
}

dump(sw2.elapsed())

[ttytm]

It would require an extended test to tell if there really is significance to it. The one I wrote is to simple. If there is performance difference and it is just some nano seconds the benefits of string comparison might still outweigh it.

[JalonSolov]

A decent optimizer will reduce both checks to almost nothing... if possible.

It still might be useful to do the same checks with -prod to see what happens. Also not sure I trust stopwatch...

[ttytm]

The conditions generate the same C 0 len check... There can't be a difference.

[JalonSolov]

How about other backends?

[ttytm]

Need to check. If the optimal is not generated for other backends what I already think is that we should address it in the backends.

[spytheman]

The s == '' optimization was implemented in the C backend in #6527 in 2020-10-01.

I agree, that if it is not, it should be done for the other backends too.

Another alternative, is that it can be reimplemented in the transformer stage, since it is a very mechanical and easy to automate rule. Then all backends will get it.

[spytheman]

Good work.

[spytheman]

@medvednikov what do you think of having a vet rule, for finding s.len == 0 conditions, and suggesting s == '' instead?