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

Fix some arithmetic invalidations #37820

Merged
merged 1 commit into from
Oct 1, 2020
Merged

Fix some arithmetic invalidations #37820

merged 1 commit into from
Oct 1, 2020

Conversation

omus
Copy link
Member

@omus omus commented Sep 30, 2020

While investigating invenia/Intervals.jl#144 I noticed these invalidations:

 inserting +(a, b::T) where T<:Interval in Intervals at /Users/omus/.julia/dev/Intervals/src/interval.jl:312 invalidated:
   mt_backedges: 1: signature Tuple{typeof(+), Ptr{UInt8}, Any} triggered MethodInstance for memcpy(::Ptr{UInt8}, ::Int64, ::Ptr{UInt8}, ::Any, ::Int64) (0 children)
                 2: signature Tuple{typeof(+), Ptr{UInt8}, Any} triggered MethodInstance for pointer(::String, ::Integer) (6 children)

The use of Any stuck out to me so I looked into these and the changes PR was is the result. With these changes this invalidations no longer exist. Please let me know if I'm off base here as I am new to addressing invalidations.

@omus omus added the compiler:latency Compiler latency label Sep 30, 2020
@omus
Copy link
Member Author

omus commented Sep 30, 2020

Doctest failure looks related. Will investigate. Stacktrace:

ERROR: LoadError: TypeError: in typeassert, expected UInt64, got a value of type Int64
Stacktrace:
  [1] writeshortest(buf::Vector{UInt8}, pos::Int64, x::Float64, plus::Bool, space::Bool, hash::Bool, precision::Int64, expchar::UInt8, padexp::Bool, decchar::UInt8, typed::Bool, compact::Bool)
    @ Base.Ryu ./ryu/shortest.jl:387
  [2] show(io::IOContext{IOBuffer}, x::Float64, forceuntyped::Bool, fromprint::Bool)
    @ Base.Ryu ./ryu/Ryu.jl:115
  [3] show(io::IOContext{IOBuffer}, x::Float64)
...

base/ryu/shortest.jl Outdated Show resolved Hide resolved
@@ -92,7 +92,7 @@ String(s::CodeUnits{UInt8,String}) = s.s
## low-level functions ##

pointer(s::String) = unsafe_convert(Ptr{UInt8}, s)
pointer(s::String, i::Integer) = pointer(s)+(i-1)
pointer(s::String, i::Integer) = pointer(s) + (i - 1)::Integer
Copy link
Member

Choose a reason for hiding this comment

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

Sometimes these are written as Int(i)::Int - 1.

Copy link
Member Author

@omus omus Sep 30, 2020

Choose a reason for hiding this comment

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

Is there any concern about having values too large to convert to an Int? It seems unlikely that would occur but I figured I'd ask

Copy link
Member

Choose a reason for hiding this comment

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

I'd think string length is pretty safe; someone would need to have a single string that's more than half the size of memory on a 32-bit machine. I guess you could alternatively use UInt.

@omus omus force-pushed the cv/arithmetic-latency branch from 72ebdec to a0f4c34 Compare September 30, 2020 21:13
@omus
Copy link
Member Author

omus commented Oct 1, 2020

I think there is nothing left to do here. I'll wait to merge until I get approval

@omus omus merged commit 2fe2b43 into master Oct 1, 2020
@omus omus deleted the cv/arithmetic-latency branch October 1, 2020 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler:latency Compiler latency
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants