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

Base.summarysize not properly accounting for Vectors containing certain types #40773

Closed
millerjoey opened this issue May 10, 2021 · 3 comments · Fixed by #41492
Closed

Base.summarysize not properly accounting for Vectors containing certain types #40773

millerjoey opened this issue May 10, 2021 · 3 comments · Fixed by #41492
Assignees
Labels
bug Indicates an unexpected problem or unintended behavior

Comments

@millerjoey
Copy link

millerjoey commented May 10, 2021

Hey all. On Julia 1.5.3:

julia> versioninfo()
Julia Version 1.5.3
Commit 788b2c77c1 (2020-11-09 13:37 UTC)
Platform Info:
  OS: Linux (x86_64-pc-linux-gnu)
  CPU: Intel(R) Core(TM) i7-8565U CPU @ 1.80GHz
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-9.0.1 (ORCJIT, skylake)
Environment:
  JULIA_EDITOR = code
  JULIA_NUM_THREADS = 

it seems that Base.summarysize doesn't recurse deeply enough for Vectors containing Sets (as well as for our own user-defined type).

julia> s = Set(1:100)
Set{Int64} with 100 elements:
  2
  89
  11
  29
  74
  57
  31
  78
  70
  33
  96
   
julia> c = [s]
1-element Array{Set{Int64},1}:
 Set([2, 89, 11, 29, 74, 57, 31, 78, 70, 33    19, 51, 22, 6, 24, 73, 53, 23, 27, 56])
julia> Base.summarysize(c)
48
julia> Base.summarysize(s)
4800

Array{Array{Int64}}, Array{Dict{Int64,Int64}}, and a few other permutations seem to work fine.

I dug a little bit and I think this has something to do with it and is preventing the function from accessing the set:

julia> Base.allocatedinline(Dict{Int,Int})
false

julia> Base.allocatedinline(Vector{Int})
false

julia> Base.allocatedinline(Set{Int})
true

which is defined here

allocatedinline(T::Type) = (@_pure_meta; ccall(:jl_stored_inline, Cint, (Any,), T) != Cint(0))

@JeffBezanson JeffBezanson added the bug Indicates an unexpected problem or unintended behavior label May 17, 2021
@JeffBezanson JeffBezanson self-assigned this May 17, 2021
@JeffBezanson
Copy link
Member

Great catch, you're right --- allocatedinline used to mean something didn't contain references, but that's no longer true.

@ederag
Copy link
Contributor

ederag commented Jun 12, 2021

To help others who might search for varinfo issues: it is affected as well, since varinfo uses summarysize.

@ddtarjan
Copy link

ddtarjan commented Jun 25, 2021

As a quickfix, I copied this function replacing "SimpleVector" with "Vector" so it will have a specialization for Vectors also. It helps.
May be this would be the fix for this issue?

function (ss::SummarySize)(obj::SimpleVector)
    key = pointer_from_objref(obj)
    haskey(ss.seen, key) ? (return 0) : (ss.seen[key] = true)
    size::Int = Core.sizeof(obj)
    if !isempty(obj)
        push!(ss.frontier_x, obj)
        push!(ss.frontier_i, 1)
    end
    return size
end

KristofferC pushed a commit that referenced this issue Jul 19, 2021
KristofferC pushed a commit that referenced this issue Sep 3, 2021
KristofferC pushed a commit that referenced this issue Sep 3, 2021
staticfloat pushed a commit that referenced this issue Dec 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates an unexpected problem or unintended behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants