Make match return Nullable{RegexMatch}#12033
Make match return Nullable{RegexMatch}#12033malmaud wants to merge 1 commit intoJuliaLang:masterfrom
Conversation
|
Ref #10550. But as I noted there, the compiler is pretty smart about this particular kind of type instability and I would not be surprised if it's more expensive to allocate a Nullable. |
|
Compiler cleverness or not, IMO this is a conceptually better API. @johnmyleswhite and I have also seen that the compiler can be pretty smart dealing with Nullables as well. +1 to the change here. |
|
Quick benchmark: Before: function f(N=10000)
r=r"a"
x=Vector{RegexMatch}()
for i=1:N
push!(x, match(r, "a"))
end
x
end
@time f();
3.023 milliseconds (60011 allocations: 2600 KB)After: function f(N=10000)
r=r"a"
x=Vector{Nullable{RegexMatch}}()
for i=1:N
push!(x, match(r, "a"))
end
x
end
@time f();
3.336 milliseconds (90011 allocations: 3538 KB) |
|
Does anybody have any examples with generated code to show whether or not |
|
It occurs to me that if we're going to do this, we should also change |
|
This appears to freeze during bootstrap on Win64. |
|
@tkelman Are you sure that's a problem with this PR and not an unrelated issue with appveyor? |
|
I'll build locally and check, but freezes during bootstrap are not currently expected - only freezes during test startup. |
|
A local cygwin cross-compile actually works okay, so I'll restart the appveyor build. We're using a slightly different toolchain on appveyor for various reasons, will add to my to-do list to try and reproduce via the appveyor script. |
|
FWIW, I agree that using Simon, if we solved the GC issues that cause non-bitstype immutables to have pointer indirection, would you expect that Re the |
|
Also FWIW, using a EDIT: Now including "before" and "after", since I realized my previous example was on the current implementation of Before: using NullableArrays
function f(N=10000)
r=r"a"
x=NullableArray{RegexMatch, 1}()
for i=1:N
push!(x, match(r, "a"))
end
x
end
f();
julia> @time f();
3.049 milliseconds (60025 allocations: 2633 KB)After: using NullableArrays
function f(N=10000)
r=r"a"
x=NullableArray{RegexMatch, 1}()
for i=1:N
push!(x, match(r, "a"))
end
x
end
f();
julia> @time f();
3.089 milliseconds (70025 allocations: 2945 KB)However, the time elapsed seems to be quite variable, going up to as high as ~9 ms on some of my runs. |
|
Wanted to ping this again. I know it's an obnoxious breaking change, but I don't think we want a Julia 1.0 that has a type-unstable |
|
Similarly breaking changes are discussed at #15755. Would be logical to make them all at the same time. |
|
Nullable is removed from Base |
Obviously this is a breaking change, but it has to be done at some point if we ever want a type-stable
match.