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

mpfr: speed improvements #27331

Merged
merged 2 commits into from
Jun 12, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 41 additions & 16 deletions base/mpfr.jl
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import
cosh, sinh, tanh, sech, csch, coth, acosh, asinh, atanh,
cbrt, typemax, typemin, unsafe_trunc, realmin, realmax, rounding,
setrounding, maxintfloat, widen, significand, frexp, tryparse, iszero,
isone, big, RefValue
isone, big, _string_n

import .Base.Rounding: rounding_raw, setrounding_raw

Expand All @@ -36,10 +36,11 @@ function __init__()
catch ex
Base.showerror_nostdio(ex, "WARNING: Error during initialization of module MPFR")
end
nothing
end

const ROUNDING_MODE = RefValue{Cint}(0)
const DEFAULT_PRECISION = RefValue(256)
const ROUNDING_MODE = Ref{Cint}(0) # actually an Enum, defined by `to_mpfr`
const DEFAULT_PRECISION = Ref{Int}(256)

# Basic type and initialization definitions

Expand All @@ -53,21 +54,44 @@ mutable struct BigFloat <: AbstractFloat
sign::Cint
exp::Clong
d::Ptr{Limb}
# _d::Buffer{Limb} # Julia gc handle for memory @ d
_d::String # Julia gc handle for memory @ d (optimized)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mutating immutable memory is dangerous. If you must have this add a FixedSizeBuffer type.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not that dangerous, it's only bad if this pointer gets leaked anywhere else, but it's purely internal. I mean, it's not like there's a deepcopy method definition here that assumes they can't be mutated or anything like that. No, that would be crazy talk, and definitely not a bug in this very PR. :P

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, is it a problem though? since BigFloat itself is not supposed to be mutated either, they share the same danger if someone violated that expectation

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Immutable memory is still being overwritten (since you construct it and then overwrite it with the result of an operation). I agree that it's much less likely to be harmful, since the compiler shouldn't be able to know the objectid of a newly-allocated bigfloat that gets subsequently modified (as long as the BigFloat itself isn't mutated more than once), but it does sound fairly risky.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't that just the same concern as any other usage of String?


# Not recommended for general use:
# used internally by, e.g. deepcopy
global function _BigFloat(prec::Clong, sign::Cint, exp::Clong, d::String)
# ccall-based version, inlined below
#z = new(zero(Clong), zero(Cint), zero(Clong), C_NULL, d)
#ccall((:mpfr_custom_init,:libmpfr), Cvoid, (Ptr{Limb}, Clong), d, prec) # currently seems to be a no-op in mpfr
#NAN_KIND = Cint(0)
#ccall((:mpfr_custom_init_set,:libmpfr), Cvoid, (Ref{BigFloat}, Cint, Clong, Ptr{Limb}), z, NAN_KIND, prec, d)
#return z
return new(prec, sign, exp, pointer(d), d)
end

function BigFloat()
prec = precision(BigFloat)
z = new(zero(Clong), zero(Cint), zero(Clong), C_NULL)
ccall((:mpfr_init2,:libmpfr), Cvoid, (Ref{BigFloat}, Clong), z, prec)
finalizer(cglobal((:mpfr_clear, :libmpfr)), z)
return z
prec::Clong = precision(BigFloat)
nb = ccall((:mpfr_custom_get_size,:libmpfr), Csize_t, (Clong,), prec)
nb = (nb + Core.sizeof(Limb) - 1) ÷ Core.sizeof(Limb) # align to number of Limb allocations required for this
#d = Vector{Limb}(undef, nb)
d = _string_n(nb * Core.sizeof(Limb))
EXP_NAN = Clong(1) - Clong(typemax(Culong) >> 1)
return _BigFloat(prec, one(Cint), EXP_NAN, d) # +NAN
end
end

# Not recommended for general use:
function BigFloat(prec::Clong, sign::Cint, exp::Clong, d::Ptr{Cvoid})
new(prec, sign, exp, d)
# overload the definition of unsafe_convert to ensure that `x.d` is assigned
# it may have been dropped in the event that the BigFloat was serialized
Base.unsafe_convert(::Type{Ref{BigFloat}}, x::Ptr{BigFloat}) = x
@inline function Base.unsafe_convert(::Type{Ref{BigFloat}}, x::Ref{BigFloat})
x = x[]
if x.d == C_NULL
x.d = pointer(x._d)
end
return convert(Ptr{BigFloat}, Base.pointer_from_objref(x))
end


"""
BigFloat(x)

Expand Down Expand Up @@ -738,6 +762,7 @@ function setprecision(::Type{BigFloat}, precision::Int)
throw(DomainError(precision, "`precision` cannot be less than 2."))
end
DEFAULT_PRECISION[] = precision
return precision
end

setprecision(precision::Int) = setprecision(BigFloat, precision)
Expand Down Expand Up @@ -956,11 +981,11 @@ set_emin!(x) = ccall((:mpfr_set_emin, :libmpfr), Cvoid, (Clong,), x)

function Base.deepcopy_internal(x::BigFloat, stackdict::IdDict)
haskey(stackdict, x) && return stackdict[x]
prec = precision(x)
y = BigFloat(zero(Clong), zero(Cint), zero(Clong), C_NULL)
ccall((:mpfr_init2,:libmpfr), Cvoid, (Ref{BigFloat}, Clong), y, prec)
finalizer(cglobal((:mpfr_clear, :libmpfr)), y)
ccall((:mpfr_set, :libmpfr), Int32, (Ref{BigFloat}, Ref{BigFloat}, Int32), y, x, ROUNDING_MODE[])
# d = copy(x._d)
d = x._d
d′ = GC.@preserve d unsafe_string(pointer(d), sizeof(d)) # creates a definitely-new String
y = _BigFloat(x.prec, x.sign, x.exp, d′)
#ccall((:mpfr_custom_move,:libmpfr), Cvoid, (Ref{BigFloat}, Ptr{Limb}), y, d) # unnecessary
stackdict[x] = y
return y
end
Expand Down
18 changes: 9 additions & 9 deletions stdlib/Serialization/src/Serialization.jl
Original file line number Diff line number Diff line change
Expand Up @@ -137,15 +137,16 @@ const REF_OBJECT_TAG = Int32(o0+13)
const FULL_GLOBALREF_TAG = Int32(o0+14)
const HEADER_TAG = Int32(o0+15)

writetag(s::IO, tag) = write(s, UInt8(tag))
writetag(s::IO, tag) = (write(s, UInt8(tag)); nothing)

function write_as_tag(s::IO, tag)
tag < VALUE_TAGS && write(s, UInt8(0))
write(s, UInt8(tag))
nothing
end

# cycle handling
function serialize_cycle(s::AbstractSerializer, x)
function serialize_cycle(s::AbstractSerializer, @nospecialize(x))
offs = get(s.table, x, -1)::Int
if offs != -1
if offs <= typemax(UInt16)
Expand Down Expand Up @@ -225,6 +226,7 @@ function serialize(s::AbstractSerializer, x::Symbol)
write(s.io, Int32(len))
end
unsafe_write(s.io, pname, len)
nothing
end

function serialize_array_data(s::IO, a)
Expand Down Expand Up @@ -290,6 +292,7 @@ function serialize(s::AbstractSerializer, ss::String)
write(s.io, Int64(len))
end
write(s.io, ss)
nothing
end

function serialize(s::AbstractSerializer, ss::SubString{String})
Expand All @@ -310,11 +313,6 @@ function serialize(s::AbstractSerializer, n::BigInt)
serialize(s, string(n, base = 62))
end

function serialize(s::AbstractSerializer, n::BigFloat)
serialize_type(s, BigFloat)
serialize(s, string(n))
end

function serialize(s::AbstractSerializer, ex::Expr)
serialize_cycle(s, ex) && return
l = length(ex.args)
Expand Down Expand Up @@ -546,6 +544,7 @@ function serialize_type_data(s, t::DataType)
end
end
end
nothing
end

function serialize(s::AbstractSerializer, t::DataType)
Expand Down Expand Up @@ -575,6 +574,7 @@ function serialize(s::AbstractSerializer, n::Int32)
writetag(s.io, INT32_TAG)
write(s.io, n)
end
nothing
end

function serialize(s::AbstractSerializer, n::Int64)
Expand All @@ -587,6 +587,7 @@ function serialize(s::AbstractSerializer, n::Int64)
writetag(s.io, INT64_TAG)
write(s.io, n)
end
nothing
end

serialize(s::AbstractSerializer, ::Type{Bottom}) = write_as_tag(s.io, BOTTOM_TAG)
Expand Down Expand Up @@ -636,6 +637,7 @@ function serialize_any(s::AbstractSerializer, @nospecialize(x))
end
end
end
nothing
end

"""
Expand Down Expand Up @@ -1177,8 +1179,6 @@ function deserialize(s::AbstractSerializer, T::Type{Dict{K,V}}) where {K,V}
return t
end

deserialize(s::AbstractSerializer, ::Type{BigFloat}) = parse(BigFloat, deserialize(s))

deserialize(s::AbstractSerializer, ::Type{BigInt}) = parse(BigInt, deserialize(s), base = 62)

function deserialize(s::AbstractSerializer, t::Type{Regex})
Expand Down
16 changes: 11 additions & 5 deletions test/mpfr.jl
Original file line number Diff line number Diff line change
Expand Up @@ -853,11 +853,17 @@ end
@test_throws ArgumentError parse(BigFloat, "1\0")

@testset "serialization (issue #12386)" begin
b = IOBuffer()
x = 2.1 * big(pi)
serialize(b, x)
seekstart(b)
@test deserialize(b) == x
b = PipeBuffer()
let x = setprecision(53) do
return 2.1 * big(pi)
end
serialize(b, x)
@test deserialize(b) == x
end
let x = BigFloat(Inf, 46)
serialize(b, x)
@test deserialize(b) == x == BigFloat(Inf, 2)
end
end
@test isnan(sqrt(BigFloat(NaN)))

Expand Down
7 changes: 7 additions & 0 deletions test/precompile.jl
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,9 @@ try
g() = override(1.0)
Test.@test g() === 2.0 # compile this
const abigfloat_f() = big"12.34"
const abigfloat_x = big"43.21"
end
""")
@test_throws ErrorException Core.kwfunc(Base.nothing) # make sure `nothing` didn't have a kwfunc (which would invalidate the attempted test)
Expand All @@ -164,6 +167,10 @@ try
@test Foo.override(1.0e0) == Float64('a')
@test Foo.override(1.0f0) == 'b'
@test Foo.override(UInt(1)) == 2

# Issue #15722
@test Foo.abigfloat_f()::BigFloat == big"12.34"
@test (Foo.abigfloat_x::BigFloat + 21) == big"64.21"
end

cachedir = joinpath(dir, "compiled", "v$(VERSION.major).$(VERSION.minor)")
Expand Down