Skip to content

Commit

Permalink
Use recommended invokelatest function instead of eval (#247)
Browse files Browse the repository at this point in the history
Using `Core.eval` is slow and often full of minefields. The `invokelatest` API is what's recommended for this purpose to avoid those.
  • Loading branch information
vtjnash authored and timholy committed Dec 23, 2019
1 parent 2d0a7f9 commit 2d99436
Showing 1 changed file with 12 additions and 16 deletions.
28 changes: 12 additions & 16 deletions src/loadsave.jl
Original file line number Diff line number Diff line change
Expand Up @@ -128,29 +128,25 @@ end
function save(df::Type{DataFormat{sym}}, f::AbstractString, data...; options...) where sym
libraries = applicable_savers(df)
checked_import(libraries[1])
Core.eval(Main, :($save($File($(DataFormat{sym}), $f),
$data...; $options...)))
return Base.invokelatest(save, File(DataFormat{sym}, f), data...; options...)
end

function savestreaming(df::Type{DataFormat{sym}}, s::IO, data...; options...) where sym
libraries = applicable_savers(df)
checked_import(libraries[1])
Core.eval(Main, :($savestreaming($Stream($(DataFormat{sym}), $s),
$data...; $options...)))
return Base.invokelatest(savestreaming, Stream(DataFormat{sym}, s), data...; options...)
end

function save(df::Type{DataFormat{sym}}, s::IO, data...; options...) where sym
libraries = applicable_savers(df)
checked_import(libraries[1])
Core.eval(Main, :($save($Stream($(DataFormat{sym}), $s),
$data...; $options...)))
return Base.invokelatest(save, Stream(DataFormat{sym}, s), data...; options...)
end

function savestreaming(df::Type{DataFormat{sym}}, f::AbstractString, data...; options...) where sym
libraries = applicable_savers(df)
checked_import(libraries[1])
Core.eval(Main, :($savestreaming($File($(DataFormat{sym}), $f),
$data...; $options...)))
return Base.invokelatest(savestreaming, File(DataFormat{sym}, f), data...; options...)
end

# do-syntax for streaming IO
Expand All @@ -168,6 +164,7 @@ end
# Handlers for formatted files/streams

for fn in (:load, :loadstreaming, :metadata)
gen2_func_name = Symbol("fileio_", fn)
@eval function $fn(@nospecialize(q::Formatted), @nospecialize(args...); @nospecialize(options...))
if unknown(q)
isfile(filename(q)) || open(filename(q)) # force systemerror
Expand All @@ -178,14 +175,13 @@ for fn in (:load, :loadstreaming, :metadata)
for library in libraries
try
Library = checked_import(library)
gen2_func_name = Symbol("fileio_" * $(string(fn)))
if isdefined(Library, gen2_func_name)
return Core.eval(Library, :($gen2_func_name($q, $args...; $options...)))
if isdefined(Library, $(QuoteNode(gen2_func_name)))
return Base.invokelatest(Library.$gen2_func_name, q, args...; options...)
end
if !has_method_from(methods(Library.$fn), Library)
throw(LoaderError(string(library), "$($fn) not defined"))
end
return Core.eval(Main, :($(Library.$fn)($q, $args...; $options...)))
return Base.invokelatest(Library.$fn, q, args...; options...)
catch e
push!(failures, (e, q))
end
Expand All @@ -195,21 +191,21 @@ for fn in (:load, :loadstreaming, :metadata)
end

for fn in (:save, :savestreaming)
gen2_func_name = Symbol("fileio_", fn)
@eval function $fn(@nospecialize(q::Formatted), @nospecialize(data...); @nospecialize(options...))
unknown(q) && throw(UnknownFormat(q))
libraries = applicable_savers(q)
failures = Any[]
for library in libraries
try
Library = checked_import(library)
gen2_func_name = Symbol("fileio_" * $(string(fn)))
if isdefined(Library, gen2_func_name)
return Core.eval(Library, :($gen2_func_name($q, $data...; $options...)))
if isdefined(Library, $(QuoteNode(gen2_func_name)))
return Base.invokelatest(Library.$gen2_func_name, q, data...; options...)
end
if !has_method_from(methods(Library.$fn), Library)
throw(WriterError(string(library), "$($fn) not defined"))
end
return Core.eval(Main, :($(Library.$fn)($q, $data...; $options...)))
return Base.invokelatest(Library.$fn, q, data...; options...)
catch e
push!(failures, (e, q))
end
Expand Down

0 comments on commit 2d99436

Please sign in to comment.