Skip to content

Commit

Permalink
Add reverse and cmp
Browse files Browse the repository at this point in the history
  • Loading branch information
jakobnissen committed Jul 27, 2024
1 parent b188cdb commit 0150982
Show file tree
Hide file tree
Showing 4 changed files with 89 additions and 13 deletions.
8 changes: 4 additions & 4 deletions docs/src/base.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ Can you define the subset of these types which indicate dense indices? I can't.
Third, it's multidimensional. It may `collect` to a `Vector` or `Matrix`.

This is not a design flaw of `SubArray` - it's a perfectly fine design choice, which enables `SubArray` to be extremely flexible and broadly useful.
Unfortunately, it also makes it nearly impossible to write robust, low-level code using `SubArray`, because it's almost imopssible not to violate the assumptions of a subset of `SubArray`s many concrete types.
Unfortunately, it also makes it nearly impossible to write robust, low-level code using `SubArray`, because it's almost impossible not to violate the assumptions of a subset of `SubArray`s many concrete types.
Practically speaking, what happens is that methods taking `SubArray` fall back to only assuming what can be assumed about `AbstractArray` - which may be inefficient, and buggy (as the recurring bugs due to assumption of one-based indexing has taught us).

In contrast, a `MemoryView{T}` is _always_ represented by exactly a `MemoryRef{T}` and an `Int` as length.
Expand All @@ -44,7 +44,7 @@ MemoryView type directly, but it's nicer to dispatch on `::MemoryKind` than on `
This may easily be added with a `GenericMemoryView` type, similar to `Memory` / `GenericMemory`.

* I can't figure out how to support reinterpreted arrays.
Any way I can think of doing so will sigificantly complicate `MemoryView`, which takes away some of
Any way I can think of doing so will significantly complicate `MemoryView`, which takes away some of
the appeal of this type's simplicity.
It's possible that reinterpreted arrays are so outside Julia's ordinary memory management
that this simply can't be done.
Expand All @@ -56,7 +56,7 @@ MemoryView type directly, but it's nicer to dispatch on `::MemoryKind` than on `

## Alternative proposal
In `examples/alternative.jl`, there is an implementation where a `MemoryView` is just a pointer and a length.
This makes it nearly identical to `Random.UnsafeView`, however, compared to `UnsafeView`, this propsal has:
This makes it nearly identical to `Random.UnsafeView`, however, compared to `UnsafeView`, this proposal has:

* The `MemoryKind` trait, useful to control dispatch to functions that can treat arrays _as being memory_
* The distinction between mutable and immutable memory views
Expand All @@ -74,5 +74,5 @@ less nicely with the Julia runtime. Also, the existing `GenericMemoryRef` is ess
using a pointer is fine, many will be written in pure Julia. There, it's less nice to have raw pointers.
* Code using pointer-based memviews must make sure to only have the views exist inside `GC.@preserve` blocks,
which is annoying and will almost certainly be violated accidentally somewhere
* We can't use advantages of the existing `Memory` infrasrtructure, e.g. having a `GenericMemRef` which supports
* We can't use advantages of the existing `Memory` infrastructure, e.g. having a `GenericMemRef` which supports
atomic memory.
2 changes: 1 addition & 1 deletion docs/src/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ data from being mutated through another variable.
### Constructing memory views
Construct memory views from `x` with `MemoryView(x)`.
MemoryViews should be constructable from any type that is stored as an array densely in memory.
It can also be conctructed from other non-array types that are represented by a chunk of memory (like a `String`).
It can also be constructed from other non-array types that are represented by a chunk of memory (like a `String`).
By default, constructors exists for the memory-backed types in Base:

```jldoctest; output=false
Expand Down
55 changes: 51 additions & 4 deletions src/basic.jl
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
memory(v::MemoryView) = v.ref.mem

function Base.setindex!(v::MutableMemoryView{T}, x, i::Int) where {T}
@boundscheck checkbounds(v, i)
xT = x isa T ? x : convert(T, x)::T
Expand All @@ -14,10 +16,18 @@ function Base.iterate(x::MemoryView, i::Int=1)
(@inbounds x[i], i + 1)
end

# Note: For zero-sized elements, this always returns 1:x.len, which may not be
# the correct indices. However, the result is indistinguishable from the "correct"
# result, so it doesn't matter
function Base.parentindices(x::MemoryView)
byte_offset = pointer(x.ref) - pointer(x.ref.mem)
elem_offset = div(byte_offset % UInt, Base.elsize(x) % UInt) % Int
(elem_offset + 1):(elem_offset + x.len)
elz = Base.elsize(x)
if iszero(elz)
1:(x.len)
else
byte_offset = pointer(x.ref) - pointer(x.ref.mem)
elem_offset = div(byte_offset % UInt, elz % UInt) % Int
(elem_offset + 1):(elem_offset + x.len)
end
end

function Base.copy(x::MemoryView)
Expand Down Expand Up @@ -113,7 +123,7 @@ const Bits =

function Base.:(==)(a::MemoryView{T}, b::MemoryView{T}) where {T <: Bits}
length(a) == length(b) || return false
T === Union{} && return true
(T === Union{} || Base.issingletontype(T)) && return true
a.ref === b.ref && return true
GC.@preserve a b begin
aptr = Ptr{Nothing}(pointer(a))
Expand All @@ -122,3 +132,40 @@ function Base.:(==)(a::MemoryView{T}, b::MemoryView{T}) where {T <: Bits}
end
iszero(y)
end

function Base.cmp(a::MemoryView{UInt8}, b::MemoryView{UInt8})
y = if a.ref !== b.ref
GC.@preserve a b begin
aptr = Ptr{Nothing}(pointer(a))
bptr = Ptr{Nothing}(pointer(b))
@ccall memcmp(
aptr::Ptr{Nothing},
bptr::Ptr{Nothing},
min(length(a), length(b))::Int,
)::Cint
end
else
Cint(0)
end
iszero(y) ? sign(length(a) - length(b)) : Int(y)
end

function Base.reverse!(mem::MutableMemoryView)
start = 1
stop = length(mem)
@inbounds for i in 1:(div(length(mem) % UInt, 2) % Int)
(mem[start], mem[stop]) = (mem[stop], mem[start])
start += 1
stop -= 1
end
mem
end

function Base.reverse(mem::MemoryView)
cp = MutableMemoryView(unsafe, copy(mem))
stop = length(cp) + 1
@inbounds for i in 1:length(cp)
cp[i] = mem[stop - i]
end
cp
end
37 changes: 33 additions & 4 deletions test/runtests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ end

s = Test.GenericString("abγδf")
@test codeunits(s) == MemoryView(s)
@test codeunits(s[2:end-2]) == MemoryView(s)[2:end-1]
@test codeunits(s[2:(end - 2)]) == MemoryView(s)[2:(end - 1)]
end

@testset "Immutable views are immutable" begin
Expand Down Expand Up @@ -301,6 +301,35 @@ end
@test findnext(==(0x04), mem, 1) == 2
@test findnext(==(0x04), mem, 3) === nothing
end

@testset "Reverse and reverse!" begin
for v in [
["a", "abc", "a", "c", "kij"],
[0x09, 0x05, 0x02, 0x01],
[1.0f0, -10.0f5, Inf32, Inf32],
[nothing, nothing, nothing],
]
@test reverse!(MemoryView(copy(v))) == MemoryView(reverse(v))
mem = MemoryView(v)
rev = reverse(mem)
@test rev.ref != mem.ref
@test rev == reverse(v)
@test_throws Exception reverse!(ImmutableMemoryView(v))
end
end

@testset "Cmp" begin
for (a, b, y) in [
([0x01], [0x00], 1),
(UInt8[], UInt8[], 0),
([0x02, 0x03], [0x02, 0x03, 0x01], -1),
([0x02, 0x03, 0x01], [0x02, 0x03], 1),
([0x9f], [0x9f], 0),
([0x01, 0x03, 0x02], [0x01, 0x04], -1),
]
@test cmp(MemoryView(a), MemoryView(b)) == y
end
end
end

@testset "Equality" begin
Expand All @@ -310,11 +339,11 @@ end
@test m1 == m2
@test m1 !== m2

m2 = m2[1:end-1]
m2 = m2[1:(end - 1)]
@test m1 != m2
m1 = m1[1:end-2]
m1 = m1[1:(end - 2)]
@test m1 != m2
m2 = m2[1:end-1]
m2 = m2[1:(end - 1)]
@test m1 == m2
end

Expand Down

2 comments on commit 0150982

@jakobnissen
Copy link
Member Author

Choose a reason for hiding this comment

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

@JuliaRegistrator
Copy link

Choose a reason for hiding this comment

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

Registration pull request updated: JuliaRegistries/General/110455

Tip: Release Notes

Did you know you can add release notes too? Just add markdown formatted text underneath the comment after the text
"Release notes:" and it will be added to the registry PR, and if TagBot is installed it will also be added to the
release that TagBot creates. i.e.

@JuliaRegistrator register

Release notes:

## Breaking changes

- blah

To add them here just re-invoke and the PR will be updated.

Tagging

After the above pull request is merged, it is recommended that a tag is created on this repository for the registered package version.

This will be done automatically if the Julia TagBot GitHub Action is installed, or can be done manually through the github interface, or via:

git tag -a v0.2.1 -m "<description of version>" 015098247ff4c3c239ae6f2938cecf203be1de22
git push origin v0.2.1

Please sign in to comment.