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

assert that input and output buffers passed to transcode! are independent #140

Merged
merged 6 commits into from
Apr 21, 2023

Conversation

baumgold
Copy link
Contributor

As suggested by @quinnj in #136 (comment)

@mkitti
Copy link
Member

mkitti commented Apr 13, 2023

This is a good basic check, but I think we might want to actually check for aliasing. The Base.dataids seems to be a way to to do this. Maybe someone like @mbauman could help update us on the latest with regard to alias detection. My understanding from JuliaLang/julia#25890 is that we could just see if the results of Base.dataids on each array are similar.

help?> Base.dataids
  Base.dataids(A::AbstractArray)

  Return a tuple of UInts that represent the mutable data segments of an array.

  Custom arrays that would like to opt-in to aliasing detection of their component parts can specialize this method to
  return the concatenation of the dataids of their component parts. A typical definition for an array that wraps a
  parent is Base.dataids(C::CustomArray) = dataids(C.parent).

For normal arrays, it seems

julia> A = rand(1024);

julia> B = @view A[1:16];

julia> Base.dataids(A)
(0x0000014201b1a080,)

julia> Base.dataids(B)
(0x0000014201b1a080,)

julia> Base.dataids(A) == Base.dataids(B)
true

julia> pointer(A)
Ptr{Float64} @0x0000014201b1a080

src/transcode.jl Outdated Show resolved Hide resolved
Co-authored-by: Mark Kittisopikul <mkitti@users.noreply.github.com>
Copy link
Member

@quinnj quinnj left a comment

Choose a reason for hiding this comment

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

Thanks! Can we add a quick test too?

@mkitti
Copy link
Member

mkitti commented Apr 14, 2023

@timholy just referred me to Base.mightalias.

help?> Base.mightalias
  Base.mightalias(A::AbstractArray, B::AbstractArray)

  Perform a conservative test to check if arrays A and B might share the same
  memory.

  By default, this simply checks if either of the arrays reference the same
  memory regions, as identified by their Base.dataids.

@mkitti
Copy link
Member

mkitti commented Apr 17, 2023

I wonder if we are going to need an unsafe_transcode!. The alias detection might be too conservative.

@baumgold
Copy link
Contributor Author

I wonder if we are going to need an unsafe_transcode!. The alias detection might be too conservative.

Agreed, added.

@baumgold baumgold force-pushed the identity_validation branch from 4f2339d to 7e8b77b Compare April 18, 2023 11:47
@baumgold baumgold force-pushed the identity_validation branch from 7e8b77b to 2eec770 Compare April 18, 2023 12:30
@quinnj quinnj merged commit 8423b71 into JuliaIO:master Apr 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants