Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: refactor "unreached region elimination" pass #45098

Closed
wants to merge 2 commits into from

Conversation

aviatesk
Copy link
Member

@aviatesk aviatesk commented Apr 26, 2022

Separated from #43999.

This "refactoring" is composed of:

  • move "unreached region elimination" pass into convert_to_ircode
  • mark SLOT_USEDUNDEF during abstract interpretation
  • some NFC refactoring for convert_to_ircode (now this part is pre-merged by some refactoring on convert_to_ircode #45205)

xref: #43999 (comment)

@aviatesk aviatesk changed the title refactor unreachability analysis refactor reachability analysis Apr 26, 2022
@aviatesk aviatesk changed the title refactor reachability analysis wip: refactor reachability analysis Apr 26, 2022
@aviatesk aviatesk added the DO NOT MERGE Do not merge this PR! label Apr 26, 2022
@aviatesk
Copy link
Member Author

There still seems to be regressions in the optimizer, probably around TypedSlot/PiNode generation. Will dig into this more.

@aviatesk aviatesk force-pushed the avi/unreachability branch from ec8b321 to 938efec Compare May 4, 2022 02:16
@aviatesk aviatesk removed the DO NOT MERGE Do not merge this PR! label May 4, 2022
@aviatesk
Copy link
Member Author

aviatesk commented May 4, 2022

@nanosoldier runbenchmarks(!"scalar", vs=":master")

@nanosoldier
Copy link
Collaborator

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

@aviatesk aviatesk force-pushed the avi/unreachability branch 2 times, most recently from c9daf3c to 41ce62d Compare May 4, 2022 10:32
@aviatesk aviatesk changed the title wip: refactor reachability analysis RFC: refactor reachability analysis May 4, 2022
@aviatesk
Copy link
Member Author

aviatesk commented May 4, 2022

@nanosoldier runbenchmarks(!"scalar", vs=":master")

@aviatesk aviatesk force-pushed the avi/unreachability branch from 41ce62d to 56ab8f0 Compare May 4, 2022 11:45
Comment on lines +596 to +611
if oldidx < length(labelmap)
changemap[oldidx] != 0 && (changemap[oldidx+1] = changemap[oldidx])
if coverage && labelmap[oldidx] != 0
labelmap[oldidx + 1] = labelmap[oldidx]
end
changemap[oldidx] = -1
coverage && (labelmap[oldidx] = -1)
end
# TODO: It would be more efficient to do this in bulk
deleteat!(code, idx)
deleteat!(codelocs, idx)
deleteat!(ssavaluetypes, idx)
deleteat!(stmtinfo, idx)
deleteat!(ssaflags, idx)
oldidx += 1
continue
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Keno could I ask your help here?

This PR is required for the per-BB state PR and almost complete, except these changes seem to cause some bugs within compute_basic_blocks. With the following MRE, now compute_basic_blocks at the end of convert_to_ircode results in OutOfMemoryError:

[CLICKME] MRE
using FileWatching.Pidfile

using Test

using Base.Filesystem: File
using FileWatching.Pidfile: iswindows,
    write_pidfile, parse_pidfile,
    isvalidpid, stale_pidfile,
    tryopen_exclusive, open_exclusive

# helper utilities
struct MemoryFile <: Base.AbstractPipe
    io::IOBuffer
    mtime::Float64
end
Base.pipe_reader(io::MemoryFile) = io.io
Base.Filesystem.mtime(io::MemoryFile) = io.mtime

# set the process umask so we can test the behavior of
# open mask without interference from parent's state
# and create a test environment temp directory
umask(new_mask) = ccall((@static iswindows() ? :_umask : :umask), Cint, (Cint,), new_mask)

code_typed() do
    @testset "Pidfile.jl" begin
    old_umask = umask(0o002)
    try
        mktempdir() do dir
            cd(dir) do

    # now start tests definitions:

    @testset "validpid" begin
        mypid = getpid() % Cuint
        @test isvalidpid(gethostname(), mypid)
        @test isvalidpid("", mypid)
        @test !isvalidpid("", 0 % Cuint)
        @test isvalidpid("NOT" * gethostname(), mypid)
        @test isvalidpid("NOT" * gethostname(), 0 % Cuint)
        @test isvalidpid("NOT" * gethostname(), -1 % Cuint)
        if !iswindows()
            @test isvalidpid("", 1 % Cuint)
            @test !isvalidpid("", -1 % Cuint)
            @test !isvalidpid("", -mypid)
        end
    end

    @testset "write_pidfile" begin
        buf = IOBuffer()
        pid, host, age = 0, "", 123
        pid2, host2, age2 = parse_pidfile(MemoryFile(seekstart(buf), time() - age))
        @test pid == pid2
        @test host == host2
        @test age  age2 atol=5

        host = " host\r\n"
        write(buf, "-1 $host")
        pid2, host2, age2 = parse_pidfile(MemoryFile(seekstart(buf), time() - age))
        @test pid == pid2
        @test host == host2
        @test age  age2 atol=5
        truncate(seekstart(buf), 0)

        pid, host = getpid(), gethostname()
        write_pidfile(buf, pid)
        @test read(seekstart(buf), String) == "$pid $host"
        pid2, host2, age2 = parse_pidfile(MemoryFile(seekstart(buf), time() - age))
        @test pid == pid2
        @test host == host2
        @test age  age2 atol=5
        truncate(seekstart(buf), 0)

        @testset "parse_pidfile" begin
            age = 0
            @test parse_pidfile("nonexist") === (Cuint(0), "", 0.0)
            open(io -> write_pidfile(io, pid), "pidfile", "w")
            pid2, host2, age2 = parse_pidfile("pidfile")
            @test pid == pid2
            @test host == host2
            @test age  age2 atol=10
            rm("pidfile")
        end
    end

    @assert !ispath("pidfile")
    @testset "open_exclusive" begin
        f = open_exclusive("pidfile")::File
        try
            # check that f is open and read-writable
            @test isfile("pidfile")
            @test filemode("pidfile") & 0o777 == 0o444
            @test filemode(f) & 0o777 == 0o444
            @test filesize(f) == 0
            @test write(f, "a") == 1
            @test filesize(f) == 1
            @test read(seekstart(f), String) == "a"
            chmod("pidfile", 0o600)
            @test filemode(f) & 0o777 == (iswindows() ? 0o666 : 0o600)
        finally
            close(f)
        end

        # release the pidfile after a short delay
        deleted = false
        rmtask = @async begin
            sleep(3)
            rm("pidfile")
            deleted = true
        end
        isdefined(Base, :errormonitor) && Base.errormonitor(rmtask)
        @test isfile("pidfile")
        @test !deleted

        # open the pidfile again (should wait for it to disappear first)
        t = @elapsed f2 = open_exclusive(joinpath(dir, "pidfile"))::File
        try
            @test deleted
            @test isfile("pidfile")
            @test t > 2
            if t > 6
                println("INFO: watch_file optimization appears to have NOT succeeded")
            end
            @test filemode(f2) & 0o777 == 0o444
            @test filesize(f2) == 0
            @test write(f2, "bc") == 2
            @test read(seekstart(f2), String) == "bc"
            @test filesize(f2) == 2
        finally
            close(f2)
        end
        rm("pidfile")
        wait(rmtask)

        # now test with a long delay and other non-default options
        f = open_exclusive("pidfile", mode = 0o000)::File
        try
            @test filemode(f) & 0o777 == (iswindows() ? 0o444 : 0o000)
        finally
            close(f)
        end
        deleted = false
        rmtask = @async begin
            sleep(8)
            rm("pidfile")
            deleted = true
        end
        isdefined(Base, :errormonitor) && Base.errormonitor(rmtask)
        @test isfile("pidfile")
        @test !deleted
        # open the pidfile again (should wait for it to disappear first)
        t = @elapsed f2 = open_exclusive("pidfile", mode = 0o777, poll_interval = 1.0)::File
        try
            @test deleted
            @test isfile("pidfile")
            @test filemode(f2) & 0o777 == (iswindows() ? 0o666 : 0o775)
            @test write(f2, "def") == 3
            @test read(seekstart(f2), String) == "def"
            @test t > 7
        finally
            close(f2)
        end
        rm("pidfile")
        wait(rmtask)

        @testset "test for wait == false cases" begin
            f = open_exclusive("pidfile", wait=false)
            @test isfile("pidfile")
            close(f)
            rm("pidfile")

            f = open_exclusive("pidfile")::File
            deleted = false
            rmtask = @async begin
                sleep(2)
                @test Pidfile.tryrmopenfile("pidfile")
                deleted = true
            end
            isdefined(Base, :errormonitor) && Base.errormonitor(rmtask)

            t1 = time()
            @test_throws ErrorException open_exclusive("pidfile", wait=false)
            @test time()-t1  0 atol=1

            sleep(1)
            @test !deleted

            t1 = time()
            @test_throws ErrorException open_exclusive("pidfile", wait=false)
            @test time()-t1  0 atol=1

            wait(rmtask)
            @test deleted
            t = @elapsed f2 = open_exclusive("pidfile", wait=false)::File
            @test isfile("pidfile")
            @test t  0 atol=1
            close(f)
            close(f2)
            rm("pidfile")
        end
    end

    @assert !ispath("pidfile")
    @testset "open_exclusive: break lock" begin
        # test for stale_age
        t = @elapsed f = open_exclusive("pidfile", poll_interval=3, stale_age=10)::File
        try
            write_pidfile(f, getpid())
        finally
            close(f)
        end
        @test t < 2
        t = @elapsed f = open_exclusive("pidfile", poll_interval=3, stale_age=1)::File
        close(f)
        @test 20 < t < 50
        rm("pidfile")

        t = @elapsed f = open_exclusive("pidfile", poll_interval=3, stale_age=10)::File
        close(f)
        @test t < 2
        t = @elapsed f = open_exclusive("pidfile", poll_interval=3, stale_age=10)::File
        close(f)
        @test 8 < t < 20
        rm("pidfile")
    end

    @testset "open_exclusive: other errors" begin
        error = @test_throws(Base.IOError, open_exclusive("nonexist/folder"))
        @test error.value.code == Base.UV_ENOENT

        error = @test_throws(Base.IOError, open_exclusive(""))
        @test error.value.code == Base.UV_ENOENT
    end

    @assert !ispath("pidfile")
    @testset "mkpidlock" begin
        lockf = mkpidlock("pidfile")
        @test lockf.update === nothing
        waittask = @async begin
            sleep(3)
            cd(homedir()) do
                return close(lockf)
            end
        end
        isdefined(Base, :errormonitor) && Base.errormonitor(waittask)

        # mkpidlock with no waiting
        t = @elapsed @test_throws ErrorException mkpidlock("pidfile", wait=false)
        @test t  0 atol=1

        t = @elapsed lockf1 = mkpidlock(joinpath(dir, "pidfile"))
        @test t > 2
        @test istaskdone(waittask) && fetch(waittask)
        @test !close(lockf)
        finalize(lockf1)
        t = @elapsed lockf2 = mkpidlock("pidfile")
        @test t < 2
        @test !close(lockf1)

        # test manual breakage of the lock
        # is correctly handled
        @test Pidfile.tryrmopenfile("pidfile")
        t = @elapsed lockf3 = mkpidlock("pidfile")
        @test t < 2
        @test isopen(lockf2.fd)
        @test !close(lockf2)
        @test !isopen(lockf2.fd)
        @test isfile("pidfile")
        @test close(lockf3)
        @test !isfile("pidfile")

        # Just for coverage's sake, run a test with do-block syntax
        lock_times = Float64[]
        t_loop = @async begin
            for idx in 1:100
                t = @elapsed mkpidlock("do_block_pidfile") do
                    # nothing
                end
                sleep(0.01)
                push!(lock_times, t)
            end
        end
        isdefined(Base, :errormonitor) && Base.errormonitor(t_loop)
        mkpidlock("do_block_pidfile") do
            sleep(3)
        end
        wait(t_loop)
        @test maximum(lock_times) > 2
        @test minimum(lock_times) < 1
    end

    @assert !ispath("pidfile")
    @testset "mkpidlock update" begin
        lockf = mkpidlock("pidfile")
        @test lockf.update === nothing
        new = mtime(lockf.fd)
        @test new  time() atol=1
        sleep(1)
        @test mtime(lockf.fd) == new
        touch(lockf)
        old, new = new, mtime(lockf.fd)
        @test new != old
        @test new  time() atol=1
        close(lockf)

        lockf = mkpidlock("pidfile"; refresh=0.2)
        new = mtime(lockf.fd)
        @test new  time() atol=1
        for i = 1:10
            sleep(0.5)
            old, new = new, mtime(lockf.fd)
            @test new != old
            @test new  time() atol=1
        end
        @test isopen(lockf.update::Timer)
        close(lockf)
        @test !isopen(lockf.update::Timer)

        lockf = mkpidlock("pidfile"; stale_age=10)
        @test lockf.update isa Timer
        close(lockf.update) # simulate a finalizer running in an undefined order
        close(lockf)
    end

    @assert !ispath("pidfile")
    @testset "mkpidlock for child" begin
        proc = open(`cat`, "w", devnull)
        lock = mkpidlock("pidfile", proc)
        @test isopen(lock.fd)
        @test isfile("pidfile")
        close(proc)
        @test success(proc)
        sleep(1) # give some time for the other task to finish releasing the lock resources
        @test !isopen(lock.fd)
        @test !isfile("pidfile")

        error = @test_throws Base.IOError mkpidlock("pidfile", proc)
        @test error.value.code == Base.UV_ESRCH
    end

    @assert !ispath("pidfile-2")
    @testset "mkpidlock non-blocking stale lock break" begin
        # mkpidlock with no waiting
        lockf = mkpidlock("pidfile-2", wait=false)
        @test lockf.update === nothing

        sleep(1)
        t = @elapsed @test_throws ErrorException mkpidlock("pidfile-2", wait=false, stale_age=1, poll_interval=1, refresh=0)
        @test t  0 atol=1

        sleep(5)
        t = @elapsed (lockf2 = mkpidlock("pidfile-2", wait=false, stale_age=.1, poll_interval=1, refresh=0))
        @test t  0 atol=1
        close(lockf)
        close(lockf2)
    end

    end; end # cd(tempdir)
    finally
        umask(old_umask)
    end; end # testset
end
ERROR: OutOfMemoryError()
Stacktrace:
  [1] _growbeg!
    @ ./array.jl:1004 [inlined]
  [2] _growbeg0!
    @ ./bitset.jl:120 [inlined]
  [3] _setint!
    @ ./bitset.jl:100 [inlined]
  [4] push!
    @ ./bitset.jl:273 [inlined]
  [5] basic_blocks_starts(stmts::Vector{Any})
    @ Core.Compiler ./compiler/ssair/ir.jl:56
  [6] compute_basic_blocks(stmts::Vector{Any})
    @ Core.Compiler ./compiler/ssair/ir.jl:88
  [7] convert_to_ircode(ci::Core.CodeInfo, sv::Core.Compiler.OptimizationState)
    @ Core.Compiler ./compiler/optimize.jl:661
  [8] run_passes(ci::Core.CodeInfo, sv::Core.Compiler.OptimizationState, caller::Core.Compiler.InferenceResult)
    @ Core.Compiler ./compiler/optimize.jl:548
  [9] optimize
    @ ./compiler/optimize.jl:517 [inlined]
  ...

Even if we changed this BitSet to IdSet{Int}, then the out of memory error won't happen bug another BoundsError appears...

jump_dests = BitSet()

ERROR: BoundsError: attempt to access 6-element Vector{Core.Compiler.BasicBlock} at index [7]
Stacktrace:
  [1] getindex
    @ ./essentials.jl:13 [inlined]
  [2] compute_basic_blocks(stmts::Vector{Any})
    @ Core.Compiler ./compiler/ssair/ir.jl:109
  [3] convert_to_ircode(ci::Core.CodeInfo, sv::Core.Compiler.OptimizationState)
    @ Core.Compiler ./compiler/optimize.jl:661
  [4] run_passes(ci::Core.CodeInfo, sv::Core.Compiler.OptimizationState, caller::Core.Compiler.InferenceResult)
    @ Core.Compiler ./compiler/optimize.jl:548

and so I guess we are doing something wrong during renumbering?

@nanosoldier
Copy link
Collaborator

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

aviatesk added 2 commits May 7, 2022 10:05
`SLOT_USEDUNDEF` is such a flag that has the following meaning:
> slot has uses that might raise UndefVarError

Since `Expr(:isdefined, slot)` doesn't raise `UndefVarError`, it sounds
incorrect conceptually to look for it when folding a `:isdefined`
expression to `true`.
@aviatesk aviatesk force-pushed the avi/unreachability branch from a3e08b5 to 0044c53 Compare May 7, 2022 07:07
@aviatesk aviatesk changed the title RFC: refactor reachability analysis WIP: refactor reachability analysis May 9, 2022
@aviatesk aviatesk changed the title WIP: refactor reachability analysis WIP: refactor "unreached region elimination" pass May 9, 2022
@aviatesk aviatesk closed this Jun 3, 2022
@aviatesk aviatesk deleted the avi/unreachability branch June 3, 2022 02:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants