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

Fast versions of isequal() and isless() for Nullables #18304

Merged
merged 4 commits into from
Sep 20, 2016
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
84 changes: 78 additions & 6 deletions base/nullable.jl
Original file line number Diff line number Diff line change
Expand Up @@ -63,16 +63,88 @@ get(x::Nullable) = x.isnull ? throw(NullException()) : x.value

isnull(x::Nullable) = x.isnull

function isequal(x::Nullable, y::Nullable)
if x.isnull && y.isnull
return true
elseif x.isnull || y.isnull
return false

## Operators

"""
null_safe_op(f::Any, ::Type, ::Type...)::Bool

Returns whether an operation `f` can safely be applied to any value of the passed type(s).
Returns `false` by default.

Custom types should implement methods for some or all operations `f` when applicable:
returning `true` means that the operation may be called on any bit pattern without
throwing an error (though returning invalid or nonsensical results is not a problem).
In particular, this means that the operation can be applied on the whole domain of the
type *and on uninitialized objects*. As a general rule, these properties are only true for
safe operations on `isbits` types.

Types declared as safe can benefit from higher performance for operations on nullable: by
always computing the result even for null values, a branch is avoided, which helps
vectorization.
"""
null_safe_op(f::Any, ::Type, ::Type...) = false

typealias NullSafeSignedInts Union{Int128, Int16, Int32, Int64, Int8}
typealias NullSafeUnsignedInts Union{Bool, UInt128, UInt16, UInt32, UInt64, UInt8}
typealias NullSafeInts Union{NullSafeSignedInts, NullSafeUnsignedInts}
typealias NullSafeFloats Union{Float16, Float32, Float64}
typealias NullSafeTypes Union{NullSafeInts, NullSafeFloats}
Copy link
Contributor

@eschnett eschnett Aug 31, 2016

Choose a reason for hiding this comment

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

You could also add Rational{T} and Complex{T} if T <: NullSafeTypes. If you are interested in vectorization, then respective Tuple and VecElement types would also be useful.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea. I can't yet define a typealias with T <: NullSafeTypes, can I? So the best strategy is probably to define null_safe_op(S<:NullSafeTypes,T<:NullSafeTypes}(::typeof(isequal), ::Type{Complex{S}}, ::Type{Complex{T}}).

Copy link
Member Author

Choose a reason for hiding this comment

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

As regards containers, I'm not sure it's a good idea to mark them as safe: this means isequal will be called even for nulls, and it potentially implies checking equality of all entries. So it will only be interesting for small tuples. Anyway, I think we should wait for a real use case combining Nullable and Tuple/VecElement before implementing this.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use a Holy trait here?

Copy link
Member Author

Choose a reason for hiding this comment

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

null_safe_op is basically a trait, but which applies to combinations of operations and types rather than only on types. Indeed, e.g. Int is safe for + but not for / (division by 0 error).

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah right, my mistake. I thought we were dispatching on the Union but that's not the case.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've added Complex and Rational. Unfortunately, there's no way to also support combinations of e.g. NullSafeTypes and Rational{<:NullSafeTypes} without defining one method for each combination. Since these cases are less unlikely to be common and performance-critical, I'll leave them out until we have a way to define these more easily (UnionAll).`

Copy link
Contributor

@TotalVerb TotalVerb Sep 3, 2016

Choose a reason for hiding this comment

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

It is possible;

typealias NullSafeParameterizedType Union{Rational, Complex}
null_safe_op{T<:NullSafeParameterizedType}(::Type{T}) = all(null_safe_op, T.types)

though admittedly not quite as nice.

Copy link
Member Author

Choose a reason for hiding this comment

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

That works for comparing Rational to Complex (probably a rare situation), but not for comparing e.g. Complex{Float64} with Float64. I'd have to define several methods for each combination.


null_safe_op{S<:NullSafeTypes,
T<:NullSafeTypes}(::typeof(isequal), ::Type{S}, ::Type{T}) = true
null_safe_op{S<:NullSafeTypes,
T<:NullSafeTypes}(::typeof(isequal), ::Type{Complex{S}}, ::Type{Complex{T}}) = true
null_safe_op{S<:NullSafeTypes,
T<:NullSafeTypes}(::typeof(isequal), ::Type{Rational{S}}, ::Type{Rational{T}}) = true

"""
isequal(x::Nullable, y::Nullable)

If neither `x` nor `y` is null, compare them according to their values
(i.e. `isequal(get(x), get(y))`). Else, return `true` if both arguments are null,
and `false` if one is null but not the other: nulls are considered equal.
"""
@inline function isequal{S,T}(x::Nullable{S}, y::Nullable{T})
if null_safe_op(isequal, S, T)
(x.isnull & y.isnull) | (!x.isnull & !y.isnull & isequal(x.value, y.value))
else
return isequal(x.value, y.value)
(x.isnull & y.isnull) || (!x.isnull & !y.isnull && isequal(x.value, y.value))
end
end

isequal(x::Nullable{Union{}}, y::Nullable{Union{}}) = true
isequal(x::Nullable{Union{}}, y::Nullable) = y.isnull
isequal(x::Nullable, y::Nullable{Union{}}) = x.isnull

null_safe_op{S<:NullSafeTypes,
T<:NullSafeTypes}(::typeof(isless), ::Type{S}, ::Type{T}) = true
null_safe_op{S<:NullSafeTypes,
T<:NullSafeTypes}(::typeof(isless), ::Type{Complex{S}}, ::Type{Complex{T}}) = true
null_safe_op{S<:NullSafeTypes,
T<:NullSafeTypes}(::typeof(isless), ::Type{Rational{S}}, ::Type{Rational{T}}) = true

"""
isless(x::Nullable, y::Nullable)
Copy link
Contributor

@tkelman tkelman Sep 16, 2016

Choose a reason for hiding this comment

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

was this elsewhere in helpdb or the rst? should it be added to the rst if not?

edit: likewise with isequal


If neither `x` nor `y` is null, compare them according to their values
(i.e. `isless(get(x), get(y))`). Else, return `true` if only `y` is null, and `false`
otherwise: nulls are always considered greater than non-nulls, but not greater than
another null.
"""
@inline function isless{S,T}(x::Nullable{S}, y::Nullable{T})
# NULL values are sorted last
if null_safe_op(isless, S, T)
(!x.isnull & y.isnull) | (!x.isnull & !y.isnull & isless(x.value, y.value))
else
(!x.isnull & y.isnull) || (!x.isnull & !y.isnull && isless(x.value, y.value))
end
end

isless(x::Nullable{Union{}}, y::Nullable{Union{}}) = false
isless(x::Nullable{Union{}}, y::Nullable) = false
isless(x::Nullable, y::Nullable{Union{}}) = !x.isnull

==(x::Nullable, y::Nullable) = throw(NullException())

const nullablehash_seed = UInt === UInt64 ? 0x932e0143e51d0171 : 0xe51d0171
Expand Down
12 changes: 12 additions & 0 deletions doc/stdlib/base.rst
Original file line number Diff line number Diff line change
Expand Up @@ -263,12 +263,24 @@ All Objects

Scalar types generally do not need to implement ``isequal`` separate from ``==``\ , unless they represent floating-point numbers amenable to a more efficient implementation than that provided as a generic fallback (based on ``isnan``\ , ``signbit``\ , and ``==``\ ).

.. function:: isequal(x::Nullable, y::Nullable)

.. Docstring generated from Julia source

If neither ``x`` nor ``y`` is null, compare them according to their values (i.e. ``isequal(get(x), get(y))``\ ). Else, return ``true`` if both arguments are null, and ``false`` if one is null but not the other: nulls are considered equal.

.. function:: isless(x, y)

.. Docstring generated from Julia source

Test whether ``x`` is less than ``y``\ , according to a canonical total order. Values that are normally unordered, such as ``NaN``\ , are ordered in an arbitrary but consistent fashion. This is the default comparison used by ``sort``\ . Non-numeric types with a canonical total order should implement this function. Numeric types only need to implement it if they have special values such as ``NaN``\ .

.. function:: isless(x::Nullable, y::Nullable)

.. Docstring generated from Julia source

If neither ``x`` nor ``y`` is null, compare them according to their values (i.e. ``isless(get(x), get(y))``\ ). Else, return ``true`` if only ``y`` is null, and ``false`` otherwise: nulls are always considered greater than non-nulls, but not greater than another null.

.. function:: ifelse(condition::Bool, x, y)

.. Docstring generated from Julia source
Expand Down
2 changes: 1 addition & 1 deletion doc/stdlib/stacktraces.rst
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
* ``func::Symbol``

The name of the function containing the execution context.
* ``linfo::Nullable{MethodInstance}``
* ``linfo::Nullable{Core.MethodInstance}``

The MethodInstance containing the execution context (if it could be found).
* ``file::Symbol``
Expand Down
102 changes: 68 additions & 34 deletions test/nullable.jl
Original file line number Diff line number Diff line change
Expand Up @@ -174,40 +174,6 @@ end

@test isnull(Nullable())

# function isequal{S, T}(x::Nullable{S}, y::Nullable{T})
for T in types
x0 = Nullable()
x1 = Nullable{T}()
x2 = Nullable{T}()
x3 = Nullable(zero(T))
x4 = Nullable(one(T))

@test isequal(x0, x1) === true
@test isequal(x0, x2) === true
@test isequal(x0, x3) === false
@test isequal(x0, x4) === false

@test isequal(x1, x1) === true
@test isequal(x1, x2) === true
@test isequal(x1, x3) === false
@test isequal(x1, x4) === false

@test isequal(x2, x1) === true
@test isequal(x2, x2) === true
@test isequal(x2, x3) === false
@test isequal(x2, x4) === false

@test isequal(x3, x1) === false
@test isequal(x3, x2) === false
@test isequal(x3, x3) === true
@test isequal(x3, x4) === false

@test isequal(x4, x1) === false
@test isequal(x4, x2) === false
@test isequal(x4, x3) === false
@test isequal(x4, x4) === true
end

# function =={S, T}(x::Nullable{S}, y::Nullable{T})
for T in types
x0 = Nullable()
Expand Down Expand Up @@ -279,6 +245,74 @@ for T in types
@test get(x1.v, one(T)) === one(T)
end

# Operators
TestTypes = Union{Base.NullSafeTypes, BigInt, BigFloat,
Complex{Int}, Complex{Float64}, Complex{BigFloat},
Rational{Int}, Rational{BigInt}}.types
for S in TestTypes, T in TestTypes
u0 = zero(S)
u1 = one(S)
if S <: AbstractFloat
u2 = S(NaN)
elseif S <: Complex && S.parameters[1] <: AbstractFloat
u2 = S(NaN, NaN)
else
u2 = u1
end

v0 = zero(T)
v1 = one(T)
if T <: AbstractFloat
v2 = T(NaN)
elseif T <: Complex && T.parameters[1] <: AbstractFloat
v2 = T(NaN, NaN)
else
v2 = v1
end

for u in (u0, u1, u2), v in (v0, v1, v2)
# function isequal(x::Nullable, y::Nullable)
@test isequal(Nullable(u), Nullable(v)) === isequal(u, v)
@test isequal(Nullable(u), Nullable(u)) === true
@test isequal(Nullable(v), Nullable(v)) === true

@test isequal(Nullable(u), Nullable(v, true)) === false
@test isequal(Nullable(u, true), Nullable(v)) === false
@test isequal(Nullable(u, true), Nullable(v, true)) === true

@test isequal(Nullable(u), Nullable{T}()) === false
@test isequal(Nullable{S}(), Nullable(v)) === false
@test isequal(Nullable{S}(), Nullable{T}()) === true

@test isequal(Nullable(u), Nullable()) === false
@test isequal(Nullable(), Nullable(v)) === false
@test isequal(Nullable{S}(), Nullable()) === true
@test isequal(Nullable(), Nullable{T}()) === true
@test isequal(Nullable(), Nullable()) === true

# function isless(x::Nullable, y::Nullable)
if S <: Real && T <: Real
@test isless(Nullable(u), Nullable(v)) === isless(u, v)
@test isless(Nullable(u), Nullable(u)) === false
@test isless(Nullable(v), Nullable(v)) === false

@test isless(Nullable(u), Nullable(v, true)) === true
@test isless(Nullable(u, true), Nullable(v)) === false
@test isless(Nullable(u, true), Nullable(v, true)) === false

@test isless(Nullable(u), Nullable{T}()) === true
@test isless(Nullable{S}(), Nullable(v)) === false
@test isless(Nullable{S}(), Nullable{T}()) === false

@test isless(Nullable(u), Nullable()) === true
@test isless(Nullable(), Nullable(v)) === false
@test isless(Nullable{S}(), Nullable()) === false
@test isless(Nullable(), Nullable{T}()) === false
@test isless(Nullable(), Nullable()) === false
end
end
end

# issue #9462
for T in types
@test isa(convert(Nullable{Number}, Nullable(one(T))), Nullable{Number})
Expand Down