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

Loosen nanosecond resolution restriction (round 2) #23

Merged
merged 3 commits into from
Oct 6, 2016
Merged

Conversation

jrevels
Copy link
Member

@jrevels jrevels commented Sep 30, 2016

Re-implements #12 on top of master, introducing BenchmarkTools.save and BenchmarkTools.load as backwards-compatible (de)serialization wrappers around JLD's save/load methods.

Unfortunately, the new SerializationTests is throwing an error on the JLD side:

julia> include("test/SerializationTests.jl")
WARNING: type JLD.AssociativeWrapper{Core.Any,Core.Any,Base.Dict{Core.Any,Core.Any}} not present in workspace; reconstructing
WARNING: Method definition (::Type{JLD.##JLD.AssociativeWrapper{Core.Any,Core.Any,Base.Dict{Core.Any,Core.Any}}#276})(Any, Any) in module JLD at /Users/jarrettrevels/.julia/v0.5/JLD/src/jld_types.jl:760 overwritten at /Users/jarrettrevels/.julia/v0.5/JLD/src/jld_types.jl:760.
ERROR: LoadError: MethodError: Cannot `convert` an object of type JLD.##JLD.AssociativeWrapper{Core.Any,Core.Any,Base.Dict{Core.Any,Core.Any}}#276 to an object of type Dict{Any,Any}
This may have arisen from a call to the constructor Dict{Any,Any}(...),
since type constructors fall back to convert methods.
 in jlconvert(::Type{BenchmarkTools.BenchmarkGroup}, ::JLD.JldFile, ::Ptr{UInt8}) at /Users/jarrettrevels/.julia/v0.5/JLD/src/jld_types.jl:510
 in read_scalar(::JLD.JldDataset, ::HDF5.HDF5Datatype, ::Type{T}) at /Users/jarrettrevels/.julia/v0.5/JLD/src/JLD.jl:418
 in read(::JLD.JldDataset) at /Users/jarrettrevels/.julia/v0.5/JLD/src/JLD.jl:390
 in read_ref(::JLD.JldFile, ::HDF5.HDF5ReferenceObj) at /Users/jarrettrevels/.julia/v0.5/JLD/src/JLD.jl:518
 in read_refs(::JLD.JldDataset, ::Type{Any}, ::Int32, ::Int32, ::Tuple{Int64}) at /Users/jarrettrevels/.julia/v0.5/JLD/src/JLD.jl:491
 in read_array(::JLD.JldDataset, ::HDF5.HDF5Datatype, ::Int32, ::Int32, ::Tuple{Int64}) at /Users/jarrettrevels/.julia/v0.5/JLD/src/JLD.jl:427
 in read(::JLD.JldDataset) at /Users/jarrettrevels/.julia/v0.5/JLD/src/JLD.jl:392
 in read_ref(::JLD.JldFile, ::HDF5.HDF5ReferenceObj) at /Users/jarrettrevels/.julia/v0.5/JLD/src/JLD.jl:518
 in jlconvert(::Type{JLD.##JLD.AssociativeWrapper{Core.Any,Core.Any,Base.Dict{Core.Any,Core.Any}}#276}, ::JLD.JldFile, ::Ptr{UInt8}) at /Users/jarrettrevels/.julia/v0.5/JLD/src/jld_types.jl:510
 in read_scalar(::JLD.JldDataset, ::HDF5.HDF5Datatype, ::Type{T}) at /Users/jarrettrevels/.julia/v0.5/JLD/src/JLD.jl:418
 in read(::JLD.JldDataset) at /Users/jarrettrevels/.julia/v0.5/JLD/src/JLD.jl:390
 in read_ref(::JLD.JldFile, ::HDF5.HDF5ReferenceObj) at /Users/jarrettrevels/.julia/v0.5/JLD/src/JLD.jl:518
 in jlconvert(::Type{BenchmarkTools.BenchmarkGroup}, ::JLD.JldFile, ::Ptr{UInt8}) at /Users/jarrettrevels/.julia/v0.5/JLD/src/jld_types.jl:510
 in read_scalar(::JLD.JldDataset, ::HDF5.HDF5Datatype, ::Type{T}) at /Users/jarrettrevels/.julia/v0.5/JLD/src/JLD.jl:418
 in read(::JLD.JldDataset) at /Users/jarrettrevels/.julia/v0.5/JLD/src/JLD.jl:390
 in read(::JLD.JldFile, ::String) at /Users/jarrettrevels/.julia/v0.5/JLD/src/JLD.jl:366
 in #jldopen#9(::Array{Any,1}, ::Function, ::JLD.##39#40{String}, ::String, ::Vararg{String,N}) at /Users/jarrettrevels/.julia/v0.5/JLD/src/JLD.jl:265
 in load(::FileIO.File{FileIO.DataFormat{:JLD}}, ::String) at /Users/jarrettrevels/.julia/v0.5/JLD/src/JLD.jl:1229
 in #load#13(::Array{Any,1}, ::Function, ::String, ::String, ::Vararg{String,N}) at /Users/jarrettrevels/.julia/v0.5/FileIO/src/loadsave.jl:45
 in load(::String, ::String) at /Users/jarrettrevels/.julia/v0.5/FileIO/src/loadsave.jl:45
 in load(::String, ::String, ::Vararg{String,N}) at /Users/jarrettrevels/.julia/v0.5/BenchmarkTools/src/serialization.jl:48
 in include_from_node1(::String) at ./loading.jl:488
 in include_from_node1(::String) at /Users/jarrettrevels/data/repos/julia5/usr/lib/julia/sys.dylib:?
while loading /Users/jarrettrevels/.julia/v0.5/BenchmarkTools/test/SerializationTests.jl, in expression starting on line 6

Seems like a scoping issue, because running the code in global scope rather than module scope fixes it (e.g. try commenting out module SerializationTests and the matching end).

@timholy Any insight on how to fix the JLD problem I'm running into?

@jrevels
Copy link
Member Author

jrevels commented Oct 3, 2016

The alternative to fixing the JLD problem is that I simply don't deal with backwards compatibility for benchmark data serialization, and just merge the picosecond resolution part of this PR now. A backwards compatibility patch could then be added sometime in the future. I feel like the negative consequences of this plan aren't too dire for the majority of people, and that the benefits of picosecond resolution would outweigh the serialization compatibility costs.

The biggest downside to this plan is that the majority of data in BaseBenchmarkReports will be un-deserializable (or will require manual conversion) until a backwards compatibility patch is made in BenchmarkTools.

Anybody opposed to this plan? If there's no opposition by 2PM ET today, I'm going to move forward on this (I would normally wait a day or so, but I'm going on vacation tomorrow).

@jrevels jrevels merged commit 185bfad into master Oct 6, 2016
@jrevels jrevels deleted the jr/resolution branch October 6, 2016 02:43
@musm
Copy link
Contributor

musm commented Oct 6, 2016

thank you!

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