Replace the slow slotname Dict with a counter ("age")-based mechanism (#307)

* Replace the slow slotname Dict with a counter ("age")-based mechanism

Co-authored-by: "Kristoffer Carlsson" <[email protected]>

* make assignment_counter an Int64 to prevent overflow

Author: timholy

src/breakpoints.jl

@@ -132,8 +132,27 @@ function prepare_slotfunction(framecode::FrameCode, body::Union{Symbol,Expr})
     for i = 1:length(slotnames)
         slotname = framecode.src.slotnames[i]
         qslotname = QuoteNode(slotname)
-        getexpr = :(something($dataname.locals[$dataname.last_reference[$qslotname]]))
-        push!(assignments, Expr(:(=), slotname, :(haskey($dataname.last_reference, $qslotname) ? $getexpr : $default)))
+        list = framecode.slotnamelists[slotname]
+        if length(list) == 1
+            maxexpr = :($dataname.last_reference[$(list[1])] > 0 ? $(list[1]) : 0)
+        else
+            maxcounter, maxidx = gensym("maxcounter"), gensym("maxidx")
+            maxexpr = quote
+                begin
+                    $maxcounter, $maxidx = 0, 0
+                    for l in $list
+                        counter = $dataname.last_reference[l]
+                        if counter > $maxcounter
+                            $maxcounter, $maxidx = counter, l
+                        end
+                    end
+                    $maxidx
+                end
+            end
+        end
+        maxexsym = gensym("slotid")
+        push!(assignments, :($maxexsym = $maxexpr))
+        push!(assignments, :($slotname = $maxexsym > 0 ? something($dataname.locals[$maxexsym]) : $default))
     end
     if ismeth
         syms = sparam_syms(framecode.scope)

src/construct.jl

@@ -265,56 +265,49 @@ function prepare_call(@nospecialize(f), allargs; enter_generated = false)
 end
 
 function prepare_framedata(framecode, argvals::Vector{Any}, lenv::SimpleVector=empty_svec, caller_will_catch_err::Bool=false)
+    src = framecode.src
+    slotnames = src.slotnames::SlotNamesType
+    ssavt = src.ssavaluetypes
+    ng, ns = isa(ssavt, Int) ? ssavt : length(ssavt::Vector{Any}), length(src.slotflags)
+    if length(junk) > 0
+        olddata = pop!(junk)
+        locals, ssavalues, sparams = olddata.locals, olddata.ssavalues, olddata.sparams
+        exception_frames, last_reference = olddata.exception_frames, olddata.last_reference
+        last_exception = olddata.last_exception
+        callargs = olddata.callargs
+        resize!(locals, ns)
+        fill!(locals, nothing)
+        resize!(ssavalues, ng)
+        # for check_isdefined to work properly, we need sparams to start out unassigned
+        resize!(sparams, 0)
+        empty!(exception_frames)
+        resize!(last_reference, ns)
+        last_exception[] = nothing
+    else
+        locals = Vector{Union{Nothing,Some{Any}}}(nothing, ns)
+        ssavalues = Vector{Any}(undef, ng)
+        sparams = Vector{Any}(undef, 0)
+        exception_frames = Int[]
+        last_reference = Vector{Int}(undef, ns)
+        callargs = Any[]
+        last_exception = Ref{Any}(nothing)
+    end
+    fill!(last_reference, 0)
     if isa(framecode.scope, Method)
-        meth, src = framecode.scope::Method, framecode.src
-        slotnames = src.slotnames::SlotNamesType
-        ssavt = src.ssavaluetypes
-        ng = isa(ssavt, Int) ? ssavt : length(ssavt::Vector{Any})
+        meth = framecode.scope::Method
         nargs, meth_nargs = length(argvals), Int(meth.nargs)
-        if length(junk) > 0
-            olddata = pop!(junk)
-            locals, ssavalues, sparams = olddata.locals, olddata.ssavalues, olddata.sparams
-            exception_frames, last_reference = olddata.exception_frames, olddata.last_reference
-            last_exception = olddata.last_exception
-            callargs = olddata.callargs
-            resize!(locals, length(src.slotflags))
-            resize!(ssavalues, ng)
-            # for check_isdefined to work properly, we need sparams to start out unassigned
-            resize!(sparams, 0)
-            empty!(exception_frames)
-            empty!(last_reference)
-            last_exception[] = nothing
-        else
-            locals = Vector{Union{Nothing,Some{Any}}}(undef, length(src.slotflags))
-            ssavalues = Vector{Any}(undef, ng)
-            sparams = Vector{Any}(undef, 0)
-            exception_frames = Int[]
-            last_reference = Dict{Symbol,Int}()
-            callargs = Any[]
-            last_exception = Ref{Any}(nothing)
-        end
-        for i = 1:meth_nargs
-            last_reference[slotnames[i]::Symbol] = i
-            if meth.isva && i == meth_nargs
-                locals[i] = nargs < i ? Some{Any}(()) : (let i=i; Some{Any}(ntuple(k->argvals[i+k-1], nargs-i+1)); end)
-                break
+        islastva = meth.isva && nargs >= meth_nargs
+        for i = 1:meth_nargs-islastva
+            if nargs >= i
+                locals[i], last_reference[i] = Some{Any}(argvals[i]), 1
+            else
+                locals[i] = Some{Any}(())
             end
-            locals[i] = nargs >= i ? Some{Any}(argvals[i]) : Some{Any}(())
         end
-        # add local variables initially undefined
-        for i = (meth_nargs+1):length(slotnames)
-            locals[i] = nothing
+        if islastva
+            locals[meth_nargs] =  (let i=meth_nargs; Some{Any}(ntuple(k->argvals[i+k-1], nargs-i+1)); end)
+            last_reference[meth_nargs] = 1
         end
-    else
-        src = framecode.src
-        locals = Vector{Union{Nothing,Some{Any}}}(undef, length(src.slotflags))  # src.slotflags is concretely typed, unlike slotnames
-        fill!(locals, nothing)
-        ssavalues = Vector{Any}(undef, length(src.code))
-        sparams = Any[]
-        exception_frames = Int[]
-        last_reference = Dict{Symbol,Int}()
-        callargs = Any[]
-        last_exception = Ref{Any}(nothing)
     end
     resize!(sparams, length(lenv))
     # Add static parameters to environment

src/interpret.jl

@@ -332,9 +332,9 @@ function do_assignment!(frame, @nospecialize(lhs), @nospecialize(rhs))
     if isa(lhs, SSAValue)
         data.ssavalues[lhs.id] = rhs
     elseif isa(lhs, SlotNumber)
+        counter = (frame.assignment_counter += 1)
         data.locals[lhs.id] = Some{Any}(rhs)
-        slotnames = code.src.slotnames::SlotNamesType
-        data.last_reference[slotnames[lhs.id]::Symbol] = lhs.id
+        data.last_reference[lhs.id] = counter
     elseif isa(lhs, GlobalRef)
         Core.eval(lhs.mod, :($(lhs.name) = $(QuoteNode(rhs))))
     elseif isa(lhs, Symbol)

src/types.jl

@@ -70,6 +70,7 @@ struct FrameCode
     src::CodeInfo
     methodtables::Vector{Union{Compiled,TypeMapEntry}} # line-by-line method tables for generic-function :call Exprs
     breakpoints::Vector{BreakpointState}
+    slotnamelists::Dict{Symbol,Vector{Int}}
     used::BitSet
     generator::Bool   # true if this is for the expression-generator of a @generated function
 end
@@ -89,8 +90,13 @@ function FrameCode(scope, src::CodeInfo; generator=false, optimize=true)
             src.code[i] = nothing
         end
     end
+    slotnamelists = Dict{Symbol,Vector{Int}}()
+    for (i, sym) in enumerate(src.slotnames)
+        list = get(slotnamelists, sym, Int[])
+        slotnamelists[sym] = push!(list, i)
+    end
     used = find_used(src)
-    framecode = FrameCode(scope, src, methodtables, breakpoints, used, generator)
+    framecode = FrameCode(scope, src, methodtables, breakpoints, slotnamelists, used, generator)
     if scope isa Method
         for bp in _breakpoints
             # Manual union splitting
@@ -151,9 +157,7 @@ struct FrameData
     exception_frames::Vector{Int}
     last_exception::Base.RefValue{Any}
     caller_will_catch_err::Bool
-    # A vector from names to the slotnumber of that name
-    # for which a reference was last encountered.
-    last_reference::Dict{Symbol,Int}
+    last_reference::Vector{Int}
     callargs::Vector{Any}  # a temporary for processing arguments of :call exprs
 end
 
@@ -176,10 +180,11 @@ mutable struct Frame
     framecode::FrameCode
     framedata::FrameData
     pc::Int
+    assignment_counter::Int64
     caller::Union{Frame,Nothing}
     callee::Union{Frame,Nothing}
 end
-Frame(framecode, framedata, pc=1, caller=nothing) = Frame(framecode, framedata, pc, caller, nothing)
+Frame(framecode, framedata, pc=1, caller=nothing) = Frame(framecode, framedata, pc, 1, caller, nothing)
 
 caller(frame) = frame.caller
 callee(frame) = frame.callee
@@ -331,7 +336,7 @@ struct BreakpointSignature <: AbstractBreakpoint
     enabled::Ref{Bool}
     instances::Vector{BreakpointRef}
 end
-same_location(bp2::BreakpointSignature, bp::BreakpointSignature) = 
+same_location(bp2::BreakpointSignature, bp::BreakpointSignature) =
     bp2.f == bp.f && bp2.sig == bp.sig && bp2.line == bp.line
 function Base.show(io::IO, bp::BreakpointSignature)
     print(io, bp.f)
@@ -369,7 +374,7 @@ struct BreakpointFileLocation <: AbstractBreakpoint
     enabled::Ref{Bool}
     instances::Vector{BreakpointRef}
 end
-same_location(bp2::BreakpointFileLocation, bp::BreakpointFileLocation) = 
+same_location(bp2::BreakpointFileLocation, bp::BreakpointFileLocation) =
     bp2.path == bp.path && bp2.abspath == bp.abspath && bp2.line == bp.line
 function Base.show(io::IO, bp::BreakpointFileLocation)
     print(io, bp.path, ':', bp.line)
@@ -378,4 +383,3 @@ function Base.show(io::IO, bp::BreakpointFileLocation)
         print(io, " [disabled]")
     end
 end
-

src/utils.jl

@@ -335,16 +335,24 @@ end
 Return the local variables as a vector of `Variable`[@ref].
 """
 function locals(frame::Frame)
-    vars = Variable[]
+    vars, var_counter = Variable[], Int[]
+    varlookup = Dict{Symbol,Int}()
     data, code = frame.framedata, frame.framecode
-    added = Set{Symbol}()
     slotnames = code.src.slotnames::SlotNamesType
-    for sym in slotnames
-        sym ∈ added && continue
-        idx = get(data.last_reference, sym, 0)
-        idx == 0 && continue
-        push!(vars, Variable(something(data.locals[idx]), sym, false))
-        push!(added, sym)
+    for (sym, counter, val) in zip(slotnames, data.last_reference, data.locals)
+        counter == 0 && continue
+        var = Variable(something(val), sym, false)
+        idx = get(varlookup, sym, 0)
+        if idx > 0
+            if counter > var_counter[idx]
+                vars[idx] = var
+                var_counter[idx] = counter
+            end
+        else
+            varlookup[sym] = length(vars)+1
+            push!(vars, var)
+            push!(var_counter, counter)
+        end
     end
     if code.scope isa Method
         syms = sparam_syms(code.scope)
@@ -424,7 +432,8 @@ function eval_code end
 eval_code(frame::Frame, command::AbstractString) = eval_code(frame, Base.parse_input_line(command))
 function eval_code(frame::Frame, expr)
     maybe_quote(x) = (isa(x, Expr) || isa(x, Symbol)) ? QuoteNode(x) : x
-
+    code = frame.framecode
+    data = frame.framedata
     isexpr(expr, :toplevel) && (expr = expr.args[end])
 
     if isexpr(expr, :toplevel)
@@ -443,10 +452,14 @@ function eval_code(frame::Frame, expr)
     j = 1
     for (i, v) in enumerate(vars)
         if v.isparam
-            frame.framedata.sparams[j] = res[i]
+            data.sparams[j] = res[i]
             j += 1
         else
-            frame.framedata.locals[frame.framedata.last_reference[v.name]] = Some{Any}(v.value isa Core.Box ? Core.Box(res[i]) : res[i])
+            slot_indices = code.slotnamelists[v.name]
+            idx = argmax(data.last_reference[slot_indices])
+            slot_idx = slot_indices[idx]
+            data.last_reference[slot_idx] = (frame.assignment_counter += 1)
+            data.locals[slot_idx] = Some{Any}(v.value isa Core.Box ? Core.Box(res[i]) : res[i])
         end
     end
     eval_res

test/breakpoints.jl

@@ -116,6 +116,24 @@ struct Squarer end
     @test !any(v->v.name == :b, var)
     @test filter(v->v.name == :a, var)[1].value == 2
 
+    # Method with local scope (two slots with same name)
+    ln = @__LINE__
+    function ftwoslots()
+        y = 1
+        z = let y = y
+                y = y + 2
+                rand()
+            end
+        y = y + 1
+        return z
+    end
+    bp = breakpoint(@__FILE__, ln+5, :(y > 2))
+    frame, bp2 = @interpret ftwoslots()
+    var = JuliaInterpreter.locals(leaf(frame))
+    @test filter(v->v.name == :y, var)[1].value == 3
+    remove(bp)
+    bp = breakpoint(@__FILE__, ln+8, :(y > 2))
+    @test isa(@interpret(ftwoslots()), Float64)
 
     # Direct return
     @breakpoint gcd(1,1) a==5