manually inline setfield! in Stateful popfirst!

[KristofferC]

Fixes #27030 (comment)

julia> chars = [rand(Char) for i in 1:10^4];

# Before
julia> @btime join(chars, "");
  4.729 ms (30019 allocations: 1004.14 KiB)

# After
julia> @btime join(chars, "");
  548.426 μs (22 allocations: 66.73 KiB)

[KristofferC]

I kinda hope that this PR is ugly enough that someone just fixes the underlying issue, heh. Is the overhead here due to the getfield overloading possibility?

[martinholters]

Looks like the union splitting prevents the inlining from happening? Manual union splitting a la

diff --git a/base/iterators.jl b/base/iterators.jl
index 1ce2d65264..8eb8cfdd2b 100644
--- a/base/iterators.jl
+++ b/base/iterators.jl
@@ -1075,7 +1075,12 @@ convert(::Type{Stateful}, itr) = Stateful(itr)
         throw(EOFError())
     else
         val, state = vs
-        s.nextvalstate = iterate(s.itr, state)
+        nvs = iterate(s.itr, state)
+        if nvs === nothing
+            s.nextvalstate = nvs
+        else
+            s.nextvalstate = nvs
+        end
         s.taken += 1
         return val
     end

is as effective as this PR (and as ugly).

[KristofferC]

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

[nanosoldier]

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

[KristofferC]

👍

[martinholters]

Maybe @Keno and @vtjnash can decide whether the optimizer can be improved with reasonable effort as a more general solution or this should be merged?

[vtjnash]

I suspect the main advantage is from removing the call to convert. The union-split above also accomplishes that for the case observed here. Although the proposal in the PR is more general.

We could also look into making the definition of Stateful more efficient by improving its memory layout and/or removing the LookAhead support.

[KristofferC]

Yes, IIRC the convert call showed up as significant on the profile print. So what do we do here now, just merge or change to Martin's version or something else?

[martinholters]

For the record: I don't think "my version" is to be preferred. I justed posted it because I found it remarkable that it makes the difference for inlining or not.

[KristofferC]

Let's merge this now to have updated performance stats, whenever inference or whatever system needs to deal with this becomes smart enough, it can be revisited.