various string search perf improvements #29678

KristofferC · 2018-10-16T21:21:23Z

Before

julia> @btime findnext(isequal('v'), "abcdefghijklmnopqrstuv", 1)
  20.982 ns (0 allocations: 0 bytes)
22

julia> @btime findnext(",", "fsdfdsfdsfsdfds, sdfsfd", 1)
  39.258 ns (0 allocations: 0 bytes)
16:16

julia> @btime split("str,str,str,str,str,str,str", ",")
  662.151 ns (10 allocations: 432 bytes)

julia> @btime findlast(",", "fsdfds,fdsfsdfdssdfsfd")
  44.284 ns (0 allocations: 0 bytes)
7:7

julia> @btime findlast(isequal(','), "fsdfds,fdsfsdfdssdfsfd")
  25.692 ns (0 allocations: 0 bytes)
7

After:

julia> @btime findnext(isequal('v'), "abcdefghijklmnopqrstuv", 1)
  11.596 ns (0 allocations: 0 bytes)
22

julia> @btime findnext(",", "fsdfdsfdsfsdfds, sdfsfd", 1)
  19.166 ns (0 allocations: 0 bytes)
16:16

julia> @btime split("str,str,str,str,str,str,str", ",")
  406.869 ns (10 allocations: 432 bytes)

julia> @btime findlast(",", "fsdfds,fdsfsdfdssdfsfd")
  19.495 ns (0 allocations: 0 bytes)
7:7

julia> @btime findlast(isequal(','), "fsdfds,fdsfsdfdssdfsfd")
  22.811 ns (0 allocations: 0 bytes)
7

Fixes #29555

KristofferC · 2018-10-16T21:21:40Z

@nanosoldier runbenchmarks("string", vs = ":master")

KristofferC · 2018-10-16T21:22:30Z

base/strings/search.jl

@@ -149,7 +153,7 @@ _nthbyte(a::Union{AbstractVector{UInt8},AbstractVector{Int8}}, i) = a[i]

 function _searchindex(s::String, t::String, i::Integer)
    # Check for fast case of a single byte
-    lastindex(t) == 1 && return something(findnext(isequal(t[1]), s, i), 0)
+    sizeof(t) == 1 && return something(findnext(isequal(first(t)), s, i), 0)


These should be equivalent, right?

I don't think it is quite:

sizeof(t) == 1 tests that t has one code unit. It should probably be written as ncodeunits(t) == 1, which just happens to be the same for String.

lastindex(t) == 1 tests that t has only one character since the last valid index is at the start of the string. It could equivalently be written as length(t) == 1 which might be a bit more efficient.

It might make sense to special-case ncodeunits(t) == 1 since that implies length(t) == 1 and is more efficient to compute, so (ncodeunits(t) == 1 || length(t) == 1) could be more efficient? Or not since it's more complicated.

Hm, but why is the comment # Check for fast case of a single byte then? I feel like the code I wrote adheres more to the comment than the previous code?

It does but I think the logic works fine for any single-char string, no?

musm · 2018-10-16T21:42:59Z

base/strings/search.jl

 nothing_sentinel(i) = i == 0 ? nothing : i

 function findnext(pred::Fix2{<:Union{typeof(isequal),typeof(==)},<:AbstractChar},
                  s::String, i::Integer)
    if i < 1 || i > sizeof(s)
        i == sizeof(s) + 1 && return nothing
-        throw(BoundsError(s, i))
+        __str_throw_boundserror(s, i)


I hope the compiler gurus can eventually fix having to manually include this non-intuitive and ugly hack that is regrettably present in a lot of base function.

~~I think this is mostly related to~~

julia/base/compiler/optimize.jl

Lines 344 to 349 in 0a685d3

elseif head === :enter

# try/catch is a couple function calls,

# but don't inline functions with try/catch

# since these aren't usually performance-sensitive functions,

# and llvm is more likely to miscompile them when these functions get large

return typemax(Int)

.

Huh? There is no try/catch here.

Sorry, my bad. I got confused.

nanosoldier · 2018-10-16T22:47:22Z

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @ararslan

KristofferC · 2018-10-16T23:52:22Z

@nanosoldier runbenchmarks(ALL, vs = ":master")

nanosoldier · 2018-10-17T06:56:34Z

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @ararslan

KristofferC · 2018-10-17T13:42:15Z

base/strings/search.jl

@@ -253,7 +257,14 @@ julia> findnext("Lang", "JuliaLang", 2)
 6:9
 ```
 """
-findnext(t::AbstractString, s::AbstractString, i::Integer) = _search(s, t, i)
+function findnext(t::AbstractString, s::AbstractString, i::Integer)
+    if sizeof(t) == 1


This cant be correct for AbstractString.

It's definitely not. What is it trying to check for?

Same as the comment above

KristofferC · 2021-04-20T11:11:59Z

Upon rethinking this, I don't think this fast path is really useful.

KristofferC commented Oct 16, 2018

View reviewed changes

KristofferC added performance Must go faster strings "Strings!" labels Oct 16, 2018

musm reviewed Oct 16, 2018

View reviewed changes

KristofferC commented Oct 17, 2018

View reviewed changes

KristofferC force-pushed the kc/search_str_perf branch 3 times, most recently from ff401e8 to 105b55e Compare October 17, 2018 16:17

various string search perf improvements

fdecddc

KristofferC force-pushed the kc/search_str_perf branch from 105b55e to fdecddc Compare October 17, 2018 20:07

KristofferC closed this Apr 20, 2021

DilumAluthge deleted the kc/search_str_perf branch August 24, 2021 05:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

various string search perf improvements #29678

various string search perf improvements #29678

KristofferC commented Oct 16, 2018 •

edited

Loading

KristofferC commented Oct 16, 2018

KristofferC Oct 16, 2018

StefanKarpinski Oct 17, 2018

KristofferC Oct 17, 2018 •

edited

Loading

StefanKarpinski Oct 17, 2018

musm Oct 16, 2018

KristofferC Oct 16, 2018 •

edited

Loading

StefanKarpinski Oct 17, 2018

JeffBezanson Oct 17, 2018

KristofferC Oct 17, 2018

nanosoldier commented Oct 16, 2018

KristofferC commented Oct 16, 2018

nanosoldier commented Oct 17, 2018

KristofferC Oct 17, 2018

StefanKarpinski Oct 17, 2018

KristofferC Oct 17, 2018

KristofferC commented Apr 20, 2021

	elseif head === :enter
	# try/catch is a couple function calls,
	# but don't inline functions with try/catch
	# since these aren't usually performance-sensitive functions,
	# and llvm is more likely to miscompile them when these functions get large
	return typemax(Int)

various string search perf improvements #29678

various string search perf improvements #29678

Conversation

KristofferC commented Oct 16, 2018 • edited Loading

KristofferC commented Oct 16, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KristofferC Oct 17, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KristofferC Oct 16, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nanosoldier commented Oct 16, 2018

KristofferC commented Oct 16, 2018

nanosoldier commented Oct 17, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KristofferC commented Apr 20, 2021

KristofferC commented Oct 16, 2018 •

edited

Loading

KristofferC Oct 17, 2018 •

edited

Loading

KristofferC Oct 16, 2018 •

edited

Loading