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

Type parameterize Core.Box #53385

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Type parameterize Core.Box #53385

wants to merge 7 commits into from

Conversation

uniment
Copy link

@uniment uniment commented Feb 19, 2024

This edit aims to take a small step toward solving some of the issues in #15276.

There are performance optimizations that do not occur when Box accesses are merely typeasserted, which instead require the box to be type-parameterized (example).

This edit type-parameterizes Core.Box, and then future improvements leveraging type annotations and inference can build on top of that.

@uniment
Copy link
Author

uniment commented Feb 19, 2024

Example of performance difference between type-asserting box vs. type-parameterizing box:

julia> VERSION
v"1.12.0-DEV.17"

julia> @btime let a::Int=0; for i=1:1000; a+=i end; ()->a+=1 end;
  4.983 μs (970 allocations: 15.16 KiB)

julia> @btime let a=Core.Box{Int}(0); for i=1:1000; a.contents+=i end; ()->a.contents+=1 end;
  4.400 ns (1 allocation: 16 bytes)

(notice microseconds vs nanoseconds timing)

@uniment uniment marked this pull request as ready for review February 21, 2024 03:10
@KristofferC
Copy link
Sponsor Member

KristofferC commented Feb 21, 2024

In order for something like this to be a good idea it feels to me the compiler would actually have to produce typed Boxes?

@uniment
Copy link
Author

uniment commented Feb 22, 2024

AsYouWishWishGIF

Before:

julia> @btime let a::Int=0; for i=1:1000; a+=i end; ()->a+=1 end;
    5.000 μs (970 allocations: 15.16 KiB)

After:

julia> @btime let a::Int=0; for i=1:1000; a+=i end; ()->a+=1 end;
    4.200 ns (1 allocation: 16 bytes)

This works for typed boxed local captures. I don't yet have a good idea of how to make typed globals match this performance. Example:

julia> a::Int = 0
       @btime for i=1:1000; global a+=1 end
    5.617 μs (1000 allocations: 15.62 KiB)

@aviatesk
Copy link
Sponsor Member

Unfortunately, it seems this change introduces a semantic change because an isbitstype field is always being isdefined.

julia> function func()
           local a::Int
           for i = 1:0
               a += i
           end
           () -> a += 1
       end;

julia> func();

julia> f = func();

on master

julia> f()
ERROR: UndefVarError: `a` not defined in local scope
Suggestion: check for an assignment to a local variable that shadows a global of the same name.
Stacktrace:
 [1] (::var"#40#41")()
   @ Main ./REPL[22]:6
 [2] top-level scope
   @ REPL[25]:1

on this PR

julia> f()
569707965

@uniment
Copy link
Author

uniment commented Feb 26, 2024

That's a good catch, and I think I have a fix. However, this semantic change I'm more worried about:

on this PR

julia> function foo()
           T = Int
           a::T = 0
           () -> a+=1
       end
ERROR: UndefVarError: `T` not defined in `Main`

This errors because in this PR, T is coded into the closure's struct definition (which is executed at global scope). The deeper problem is that a's box declaration is moved to the start of its lexical scope, before T is defined.

This can be fixed if I implement the following two changes:

  1. have T parameterize the closure's struct type during closure instantiation, instead of being hard-coded into the struct definition, and
  2. leave a::T's declaration in-place, instead of moving it to the start of its lexical scope

Change (1) will be easy, but (2) would require a change in semantics: require a::T to be declared in its parent scope before its closure is instantiated. To me this is a reasonable constraint which I have never found reason to violate (except for mutually-recursive closures, but I think Julia can have a different solution for that). Already there are various minor differences between local and global scope, and giving up a feature I dislike anyway would be a welcome entry in that list for the sake of performance. However, this would constitute a break from how lexical scoping currently behaves in Julia. (Note that JavaScript, Python, and FemtoLisp also allow a local capture to be declared after its closure, but Lua's lexical scoping does not.)

Also, while exploring this issue, I found a bug in how lexical scoping on typed captures is currently handled:

on master

julia> function bar()
           T = Int
           g = () -> a+=1
           a::T = 0
           g
       end;

julia> g = bar()
       g()
ERROR: UndefVarError: `T` not defined in `Main`

(I rank the priority on this bug very low, as declaring a capture after its closure is bad juju anyway imo.)

@uniment
Copy link
Author

uniment commented Feb 27, 2024

On second thought, there's no need for semantics to change if we do this:

  1. use box type to parameterize the closure type during closure instantiation,
  2. have a = Core.Box{Any}() declared at the start of lexical scope,
  3. introduce a_capt = false at the start of lexical scope to indicate whether a has been captured yet,
  4. at the line a::T = 0, substitute tmp=convert(T,0); if !a_capt; a=Core.Box{T}() end; a.contents=tmp, and
  5. at the closure declaration, insert a_capt = true

Seems pretty involved though, might take a while.

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.

3 participants