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

1.7 allocates where 1.6 doesn't #41488

Closed
cscherrer opened this issue Jul 6, 2021 · 11 comments
Closed

1.7 allocates where 1.6 doesn't #41488

cscherrer opened this issue Jul 6, 2021 · 11 comments

Comments

@cscherrer
Copy link

cscherrer commented Jul 6, 2021

In KeywordCalls.jl we have some tests like this:

struct Foo{N,T}
    nt::NamedTuple{N,T}
end

@kwstruct Foo(a,b)
@test 0 == @ballocated Foo((b=1,a=2))

The @kwstruct Foo(a,b) expands to

julia> @macroexpand @kwstruct Foo(a,b)
quote
    function _call_in_default_order(::Type{Foo}, nt::NamedTuple{(:a, :b)})
        return Foo(NamedTuple{(:a, :b)}(nt))
    end

    function Foo(nt::NamedTuple)
        $(Expr(:meta, :inline))
        aliased = (alias)(Foo, nt)
        merged = merge((;), aliased)
        sorted = _sort(merged)
        return _call_in_default_order(Foo, sorted)
    end

    Foo(; kw...) = Foo(NamedTuple(kw))

    has_kwargs(::typeof(Foo)) = true

    Foo(nt::NamedTuple{(:a, :b), T}) where T = Foo{(:a, :b), T}(nt)

    build(::Type{Foo}, ::NamedTuple{(:a, :b), T}) where T = true
end

where _sort is like this:

@generated _sort(nt::NamedTuple{K}) where {K} = :(NamedTuple{($(QuoteNode.(sort(collect(K)))...),)}(nt))

The aliased bit doesn't really apply here (no aliases, so it defaults to identity. But just in case, it's

@inline alias(f,::Val{k}) where {k} = k

alias(f, tup::Tuple) = alias.(f, tup)

function alias(f, nt::NamedTuple{K}) where {K} 
    newnames = alias(f, Val.(K))
    NamedTuple{newnames}(values(nt))
end

These are passing in 1.6 but failing in the nightly build, and also failing in 1.7-beta. Some details here.

Here's the result in my terminal for 1.7-beta:

Precompiling project...
  12 dependencies successfully precompiled in 6 seconds
     Testing Running tests...
No Allocation: Test Failed at /home/chad/git/KeywordCalls.jl/test/runtests.jl:66
  Expression: 0 == #= /home/chad/git/KeywordCalls.jl/test/runtests.jl:66 =# @ballocated(Foo((b = 1, a = 2)))
   Evaluated: 0 == 32
Stacktrace:
 [1] macro expansion
   @ /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.7/Test/src/Test.jl:445 [inlined]
 [2] macro expansion
   @ ~/git/KeywordCalls.jl/test/runtests.jl:66 [inlined]
 [3] macro expansion
   @ /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.7/Test/src/Test.jl:1282 [inlined]
 [4] macro expansion
   @ ~/git/KeywordCalls.jl/test/runtests.jl:64 [inlined]
 [5] macro expansion
   @ /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.7/Test/src/Test.jl:1282 [inlined]
 [6] top-level scope
   @ ~/git/KeywordCalls.jl/test/runtests.jl:35

Also...

julia> versioninfo()
Julia Version 1.7.0-beta2
Commit b570546b68 (2021-06-20 06:31 UTC)
Platform Info:
  OS: Linux (x86_64-pc-linux-gnu)
  CPU: Intel(R) Core(TM) i7-7700HQ CPU @ 2.80GHz
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-12.0.0 (ORCJIT, skylake)

Also... @simeonschaub you're familiar with this code, would you have time to have a look?

@KristofferC
Copy link
Sponsor Member

What is f? It would be helpful to provide a fuller example that can be reproduced without having to dig too much inte the test files etc.

@JeffBezanson
Copy link
Sponsor Member

Would also be helpful to bisect if you can.

@cscherrer
Copy link
Author

Good point. I think I can narrow it down a little. I'll edit the OP in a minute...

@cscherrer
Copy link
Author

I've never thought about bisecting such a big codebase. I assume each step takes a while? Are there utilities to help with this? Or maybe it's just a little one-off script each time?

@mauro3
Copy link
Contributor

mauro3 commented Jul 7, 2021

Not sure this is what you asked for: git-bisect can help, e.g.: https://git-scm.com/docs/git-bisect

@cscherrer
Copy link
Author

Thanks, I've used git bisect, just not in this context. I'll post something in the Zulip rather than clutter this issue.

@JeffBezanson
Copy link
Sponsor Member

This is interesting: @allocated gives 0; @ballocated gives 32. It can be difficult to exclude 100% of allocations that may happen around testing something.

@JeffBezanson
Copy link
Sponsor Member

Also code_llvm confirms there are no heap allocations in the generated code.

@cscherrer
Copy link
Author

Oh, that's very weird. It's strange that 1.6 consistently gives 0 and 1.7 gives 32, but maybe it's a non-issue, and I should just change it to @allocated?

@vtjnash
Copy link
Sponsor Member

vtjnash commented Jul 7, 2021

We changed the @allocated macro in v1.7 to attempt to be a bit more reliable and consistent–though possibly not actually a more useful number

@cscherrer
Copy link
Author

Good to know, thanks @vtjnash

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

No branches or pull requests

6 participants