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

WIP: ImmutableArrays (using EA in Base) #44381

Closed
wants to merge 5 commits into from

Conversation

ianatol
Copy link
Member

@ianatol ianatol commented Feb 28, 2022

This rebases #42465 against #43888, which is where we are currently staging optimizations that utilize the version of EscapeAnalysis that's in Base.

CC: @aviatesk

aviatesk and others added 4 commits February 14, 2022 12:10
This commit ports [EscapeAnalysis.jl](https://github.com/aviatesk/EscapeAnalysis.jl) into Julia base.
You can find the documentation of this escape analysis at [this GitHub page](https://aviatesk.github.io/EscapeAnalysis.jl/dev/)[^1].

[^1]: The same documentation will be included into Julia's developer
      documentation by this commit.

This escape analysis will hopefully be an enabling technology for various
memory-related optimizations at Julia's high level compilation pipeline.
Possible target optimization includes alias aware SROA (JuliaLang#43888),
array SROA (JuliaLang#43909), `mutating_arrayfreeze` optimization (JuliaLang#42465),
stack allocation of mutables, finalizer elision and so on[^2].

[^2]: It would be also interesting if LLVM-level optimizations can consume
      IPO information derived by this escape analysis to broaden
      optimization possibilities.

The primary motivation for porting EA in this PR is to check its impact
on latency as well as to get feedbacks from a broader range of developers.
The plan is that we first introduce EA in this commit, and then merge the
depending PRs built on top of this commit like JuliaLang#43888, JuliaLang#43909 and JuliaLang#42465

This commit simply defines and runs EA inside Julia base compiler and
enables the existing test suite with it. In this commit, we just run EA
before inlining to generate IPO cache. The depending PRs, EA will be
invoked again after inlining to be used for various local optimizations.
Enhances SROA of mutables using the novel Julia-level escape analysis (on top of JuliaLang#43800):
1. alias-aware SROA, mutable ϕ-node elimination
2. `isdefined` check elimination
3. load-forwarding for non-eliminable but analyzable mutables

---

1. alias-aware SROA, mutable ϕ-node elimination

EA's alias analysis allows this new SROA to handle nested mutables allocations
pretty well. Now we can eliminate the heap allocations completely from
this insanely nested examples by the single analysis/optimization pass:
```julia
julia> function refs(x)
           (Ref(Ref(Ref(Ref(Ref(Ref(Ref(Ref(Ref(Ref((x))))))))))))[][][][][][][][][][]
       end
refs (generic function with 1 method)

julia> refs("julia"); @allocated refs("julia")
0
```

EA can also analyze escape of ϕ-node as well as its aliasing.
Mutable ϕ-nodes would be eliminated even for a very tricky case as like:
```julia
julia> code_typed((Bool,String,)) do cond, x
           # these allocation form multiple ϕ-nodes
           if cond
               ϕ2 = ϕ1 = Ref{Any}("foo")
           else
               ϕ2 = ϕ1 = Ref{Any}("bar")
           end
           ϕ2[] = x
           y = ϕ1[] # => x
           return y
       end
1-element Vector{Any}:
 CodeInfo(
1 ─     goto JuliaLang#3 if not cond
2 ─     goto JuliaLang#4
3 ─     nothing::Nothing
4 ┄     return x
) => Any
```

Combined with the alias analysis and ϕ-node handling above,
allocations in the following "realistic" examples will be optimized:
```julia
julia> # demonstrate the power of our field / alias analysis with realistic end to end examples
       # adapted from http://wiki.luajit.org/Allocation-Sinking-Optimization#implementation%5B
       abstract type AbstractPoint{T} end

julia> struct Point{T} <: AbstractPoint{T}
           x::T
           y::T
       end

julia> mutable struct MPoint{T} <: AbstractPoint{T}
           x::T
           y::T
       end

julia> add(a::P, b::P) where P<:AbstractPoint = P(a.x + b.x, a.y + b.y);

julia> function compute_point(T, n, ax, ay, bx, by)
           a = T(ax, ay)
           b = T(bx, by)
           for i in 0:(n-1)
               a = add(add(a, b), b)
           end
           a.x, a.y
       end;

julia> function compute_point(n, a, b)
           for i in 0:(n-1)
               a = add(add(a, b), b)
           end
           a.x, a.y
       end;

julia> function compute_point!(n, a, b)
           for i in 0:(n-1)
               a′ = add(add(a, b), b)
               a.x = a′.x
               a.y = a′.y
           end
       end;

julia> compute_point(MPoint, 10, 1+.5, 2+.5, 2+.25, 4+.75);

julia> compute_point(MPoint, 10, 1+.5im, 2+.5im, 2+.25im, 4+.75im);

julia> @allocated compute_point(MPoint, 10000, 1+.5, 2+.5, 2+.25, 4+.75)
0

julia> @allocated compute_point(MPoint, 10000, 1+.5im, 2+.5im, 2+.25im, 4+.75im)
0

julia> compute_point(10, MPoint(1+.5, 2+.5), MPoint(2+.25, 4+.75));

julia> compute_point(10, MPoint(1+.5im, 2+.5im), MPoint(2+.25im, 4+.75im));

julia> @allocated compute_point(10000, MPoint(1+.5, 2+.5), MPoint(2+.25, 4+.75))
0

julia> @allocated compute_point(10000, MPoint(1+.5im, 2+.5im), MPoint(2+.25im, 4+.75im))
0

julia> af, bf = MPoint(1+.5, 2+.5), MPoint(2+.25, 4+.75);

julia> ac, bc = MPoint(1+.5im, 2+.5im), MPoint(2+.25im, 4+.75im);

julia> compute_point!(10, af, bf);

julia> compute_point!(10, ac, bc);

julia> @allocated compute_point!(10000, af, bf)
0

julia> @allocated compute_point!(10000, ac, bc)
0
```

2. `isdefined` check elimination

This commit also implements a simple optimization to eliminate
`isdefined` call by checking load-fowardability.
This optimization may be especially useful to eliminate extra allocation
involved with a capturing closure, e.g.:
```julia
julia> callit(f, args...) = f(args...);

julia> function isdefined_elim()
           local arr::Vector{Any}
           callit() do
               arr = Any[]
           end
           return arr
       end;

julia> code_typed(isdefined_elim)
1-element Vector{Any}:
 CodeInfo(
1 ─ %1 = $(Expr(:foreigncall, :(:jl_alloc_array_1d), Vector{Any}, svec(Any, Int64), 0, :(:ccall), Vector{Any}, 0, 0))::Vector{Any}
└──      goto JuliaLang#3 if not true
2 ─      goto JuliaLang#4
3 ─      $(Expr(:throw_undef_if_not, :arr, false))::Any
4 ┄      return %1
) => Vector{Any}
```

3. load-forwarding for non-eliminable but analyzable mutables

EA also allows us to forward loads even when the mutable allocation
can't be eliminated but still its fields are known precisely.
The load forwarding might be useful since it may derive new type information
that succeeding optimization passes can use (or just because it allows
simpler code transformations down the load):
```julia
julia> code_typed((Bool,String,)) do c, s
           r = Ref{Any}(s)
           if c
               return r[]::String # adce_pass! will further eliminate this type assert call also
           else
               return r
           end
       end
1-element Vector{Any}:
 CodeInfo(
1 ─ %1 = %new(Base.RefValue{Any}, s)::Base.RefValue{Any}
└──      goto JuliaLang#3 if not c
2 ─      return s
3 ─      return %1
) => Union{Base.RefValue{Any}, String}
```

---

Please refer to the newly added test cases for more examples.
Also, EA's alias analysis already succeeds to reason about arrays, and
so this EA-based SROA will hopefully be generalized for array SROA as well.
@ianatol ianatol mentioned this pull request Feb 28, 2022
@MilesCranmer
Copy link
Member

Status?

@vtjnash
Copy link
Member

vtjnash commented Jan 30, 2024

Just doing some old PR cleanup: we no longer have Arrays

@vtjnash vtjnash closed this Jan 30, 2024
@MilesCranmer
Copy link
Member

Wait so are ImmutableArrays dead? This has been in the works since #31630 in 2019...

@fingolfin
Copy link
Contributor

I am really confused, why was this closed? @vtjnash what do you mean, we no longer have arrays? Surely that's not true as stated, I am guessing you meant something else, but what?

@vchuravy
Copy link
Member

I am guessing you meant something else, but what?

This PR would need to be completely redone to handle Memory instead of Array

@MilesCranmer
Copy link
Member

It is extremely dissappointing that this effort has been killed off, after being in the works since 2019 over many PRs, with no obvious continuation. Immutable arrays seem like such an obvious win for both performance and memory safety. I'm sad to hear there has not been more interest in this direction.


For those with immediate interest in this, if there are no other options, perhaps we can revive ReadOnlyArrays.jl. I just made a PR to add some caching information on the shapes JuliaArrays/ReadOnlyArrays.jl#13 which might get some performance wins. Perhaps some of the work done in this PR and the ones it has forked can be moved over to this or another package.

@ChrisRackauckas
Copy link
Member

That's a very uncharitable take. Clearly it would be better to build this functionality on the new Memory type #51319 because that would have less overhead and it's a simpler primitive, like how Array is now built on Memory. However, that would be a rather massive change. On top of that, there have been 2 or 3 LLVM major version upgrades since this PR was last modified, and so a lot of the LLVM parts would be rather stale as well. So it should be very clear why a code that does this would likely be better off starting almost from scratch, or using this PR more for ideas than for code.

But that isn't necessarily a bad thing. One of the big purposes of the Memory PR was to make such implementations of new array primitives, like immutable arrays and string backed arrays, much simpler to represent to the compiler so that the optimizations would be easier to achieve. As such, you can think of the merge of Memory as a major step forward in this direction, as a lot of what is done in here now is just a standard Julia thing, using the Memory type and its API. So because there have been many strides there, this PR that starts from a position that completely ignores all of the other changes in the last 2 years is simply not a good idea

@KristofferC
Copy link
Member

Yeah, closing a PR that has had no work done on it for years and is completely outdated cannot be described as "this effort has been killed off". It was already dead!

@Tokazama
Copy link
Contributor

I thought part of the reason this approach was abandoned was because we decided the memory model was inherently wrong. With the new Memory type wouldn't it be pretty simple to prototype this with something imitating the current Array implementation and add some assumed effects where appropriate?

@ChrisRackauckas
Copy link
Member

Yes exactly. So you'd just never expose setindex! and assume some effects and have a "reasonable" implementation. I'm sure the compiler team would want to add maybe a few more correctness checks but that would greatly simplify the implementation compared to an Array based one.

@MilesCranmer
Copy link
Member

Fair enough. I’m just bummed out by the news I guess.

@ChrisRackauckas
Copy link
Member

The Memory type is probably much closer to a solution already than this ever was.

@MilesCranmer
Copy link
Member

That's good. I have my fingers crossed someone can take this up. Immutable arrays would be amazing.

@vtjnash
Copy link
Member

vtjnash commented Mar 20, 2024

FWIW, I think a SubArray of a Memory looks roughly equivalent to a ReadOnlyArray already (in some ways lighter, some heavier, some more powerful, so not exactly equivalent--but mostly in the ways that SubArray already was roughly equivalent to it--but now lighter weight since it doesn't carry around the mutable Array)

I also have observed short lived Arrays get entirely removed by the compiler now and the bounds hoisted into the caller. So sometimes this doesn't need any special care anymore also, just a short lived Array.

@MilesCranmer
Copy link
Member

That's great to hear.

I think an immutable array would also be very useful from a memory safety standpoint. The dream here would be to enable packages to fully emulate rust-like ownership for regular mutable arrays.

@Tokazama
Copy link
Contributor

Rust ownership is problematic in its own ways. Tracking that sort of thing requires it's own additional compiler pass which is likely to result in nontrivial compile time costs. I think we'll find that other paradigms for memory safety will emerge in the future as we solidify when to apply assume effects and escape analysis better

@MilesCranmer
Copy link
Member

MilesCranmer commented Mar 24, 2024

I would in principle be happy with that but from your wording it sounds like this might be far into the future (?) which worries me, especially given this direction technically started 5 years ago in #31630.

I'm not expecting to convince anybody but I guess all I can say is that I think immutable arrays/memory safety should be a driving goal, rather than a side effect which emerges as a result of other developments... Funnily enough, as you might have heard, US leadership thinks so too (and makes explicit mention of Rust in the report).

Right now the only options for (partial) memory safety in Julia arrays are:

  1. Use a tuple or static array and be forced to dispatch on array size, which is impractical except for small arrays of known size
  2. Write a custom wrapper without defining setindex!, which is only safe at an interface level and doesn't let the compiler get any performance benefits

It would great if someone wants to take up translating this PR to the new Memory type. I am sure this would be hugely useful to the julia ecosystem. Of course it wouldn't be completely memory safe, but perfect is the enemy of the good, and full memory safety can be a longer term goal as it would require deeper changes.

@StefanKarpinski
Copy link
Member

Right now the only options for (partial) memory safety in Julia arrays are:

  1. Use a tuple or static array and be forced to dispatch on array size, which is impractical except for small arrays of known size
  2. Write a custom wrapper without defining setindex!, which is only safe at an interface level and doesn't let the compiler get any performance benefits

There might be some confusion here about what "memory safe" means. Julia is a memory safe language: by default, if you access out-of-bounds memory, you get an error. Like all other languages, there are ways to opt out of default memory saftey (e.g. when people write code with @inbounds they suppress bounds checks, but --check-bounds=yes overrides that and forces bounds checks anyway). Memory safety does not require immutable arrays or proving at compile time that runtime errors cannot occur. Indeed, if that were the criterion, no mainstream languages would qualify.

@MilesCranmer
Copy link
Member

MilesCranmer commented Mar 25, 2024

I might push back a bit against thinking of memory safety as a binary attribute like that... Out-of-bounds checking contributes to more memory safety than without. But there are other ways to improve here. Currently you are allowed to have multiple mutable references to arrays, and there is no way to make an immutable version. In Rust you can only have a single mutable reference but multiple immutable references. If Julia had immutable arrays we could emulate parts of this – improving memory safety.

For what it's worth I had in mind the broader definition as "safety from memory errors". See https://en.wikipedia.org/wiki/Memory_safety#Types_of_memory_errors – race conditions are included in this list. But I could totally see that some people might use "memory safety" to refer specifically to bounds checking (this is not what I meant).

@ChrisRackauckas
Copy link
Member

Types of memory errors
Many different types of memory errors can occur:[24][[25]](https://en.wikipedia.org/wiki/Memory_safety#cite_note-25)

Access errors: invalid read/write of a pointer
[Buffer overflow](https://en.wikipedia.org/wiki/Buffer_overflow) – out-of-bound writes can corrupt the content of adjacent objects, or internal data (like bookkeeping information for the [heap](https://en.wikipedia.org/wiki/Heap_memory)) or [return](https://en.wikipedia.org/wiki/Return_statement) addresses.
[Buffer over-read](https://en.wikipedia.org/wiki/Buffer_over-read) – out-of-bound reads can reveal sensitive data or help attackers bypass [address space layout randomization](https://en.wikipedia.org/wiki/Address_space_layout_randomization).
[Race condition](https://en.wikipedia.org/wiki/Race_condition) – concurrent reads/writes to shared memory
[Invalid page fault](https://en.wikipedia.org/wiki/Page_fault#Invalid) – accessing a pointer outside the virtual memory space. A null pointer dereference will often cause an exception or program termination in most environments, but can cause corruption in operating system [kernels](https://en.wikipedia.org/wiki/Kernel_(operating_system)) or systems without [memory protection](https://en.wikipedia.org/wiki/Memory_protection), or when use of the null pointer involves a large or negative offset.
Use after free – dereferencing a [dangling pointer](https://en.wikipedia.org/wiki/Dangling_pointer) storing the address of an object that has been deleted.
[Uninitialized variables](https://en.wikipedia.org/wiki/Uninitialized_variable) – a variable that has not been assigned a value is used. It may contain an undesired or, in some languages, a corrupt value.
[Null pointer](https://en.wikipedia.org/wiki/Null_pointer) dereference – dereferencing an invalid pointer or a pointer to memory that has not been allocated
[Wild pointers](https://en.wikipedia.org/wiki/Wild_pointer) arise when a pointer is used prior to initialization to some known state. They show the same erratic behaviour as dangling pointers, though they are less likely to stay undetected.
[Memory leak](https://en.wikipedia.org/wiki/Memory_leak) – when memory usage is not tracked or is tracked incorrectly
[Stack exhaustion](https://en.wikipedia.org/wiki/Stack_overflow) – occurs when a program runs out of stack space, typically because of too deep [recursion](https://en.wikipedia.org/wiki/Recursion_(computer_science)). A [guard page](https://en.wikipedia.org/wiki/Memory_protection) typically halts the program, preventing memory corruption, but functions with large [stack frames](https://en.wikipedia.org/wiki/Stack_frame) may bypass the page.
[Heap exhaustion](https://en.wikipedia.org/wiki/Out_of_memory) – the program tries to [allocate](https://en.wikipedia.org/wiki/Memory_allocation) more memory than the amount available. In some languages, this condition must be checked for manually after each allocation.
Double free – repeated calls to [free](https://en.wikipedia.org/wiki/Malloc) may prematurely free a new object at the same address. If the exact address has not been reused, other corruption may occur, especially in allocators that use [free lists](https://en.wikipedia.org/wiki/Free_list).
Invalid free – passing an invalid address to [free](https://en.wikipedia.org/wiki/Malloc) can corrupt the [heap](https://en.wikipedia.org/wiki/Heap_memory).
Mismatched free – when multiple allocators are in use, attempting to free memory with a deallocation function of a different allocator[[26]](https://en.wikipedia.org/wiki/Memory_safety#cite_note-26)
Unwanted [aliasing](https://en.wikipedia.org/wiki/Aliasing_(computing)) – when the same memory location is allocated and modified twice for unrelated purposes.

For reference, copying that down. In that, the only one that is possible is race conditions. But of course, no language can help that in general. See:

https://doc.rust-lang.org/nomicon/races.html#:~:text=However%20Rust%20does%20not%20prevent,by%20frameworks%20such%20as%20RTIC.

However Rust does not prevent general race conditions.

This is mathematically impossible in situations where you do not control the scheduler, which is true for the normal OS environment. If you do control preemption, it can be possible to prevent general races - this technique is used by frameworks such as RTIC.

I don't understand the straw man that you're trying to attack here. Everybody here is for having a form of immutable arrays. Closing this PR is getting us closer to immutable arrays, the specialized copy removal is not needed as much with new LLVM optimizations on Memory, it just needs a completely different PR for a high level type. Isn't that what you wanted?

@MilesCranmer
Copy link
Member

I’m not sure how data races are a strawman. Prevention of such issues is exactly the reason I'm interested in immutable arrays in the first place.

Anyways let me restate my argument to help clarify, as it seems we have gotten a bit caught up over details.

I would like to petition for ImmutableArray, in whatever form, to be prioritised. Increased memory safety in Julia would be hugely useful for large projects. In the past I have spent months debugging data races and it has been very painful in Julia:

Rust (Safe Rust) is guaranteed to prevent data races b/c of its ownership system (max 1 mutable ref) which is very nice for writing robust code. Note the first sentence of that page you shared:

Safe Rust guarantees an absence of data races

The part which you highlighted refers to slightly difference concept.

Improved memory safety is favored by industry which is partly why Rust has taken off (because data races are so hard to debug). I think ImmutableArray would be a huge step towards making Julia more memory safe.

the specialized copy removal is not needed as much with new LLVM optimizations on Memory, it just needs a completely different PR for a high level type. Isn't that what you wanted?

Yeah it is definitely a good thing. But I got the sense that nobody was actively working on the 'different PR for a high level type' and the effort has stalled. So I wanted to give some words of encouragment that this is a highly desired feature (hence all my comments here).

@Tokazama
Copy link
Contributor

I don't think it's so much a problem of motivation as it is priority and available time. There's still a lot of things to do with improving memory management (safety, runtime speed, GC, etc.). If someone where to make a PR putting together the complete and tested (but unoptimized) interface for an ImmutableArray that would probably get things going again. Figuring out where we need to annotate assumed effects will take some time after the fact.

If your really eager, try putting together a small personal package using the Julia version supporting Memory. You can get a lot of functionality with even a sloppy implementation of what we're discussing.

struct ImmutableArray{T, N} <: DenseArray{T, N}
    mem::Memory{T}
    size::NTuple{N, Int}
end

Base.size(x::ImmutableArray) = getfield(x, :size)

# TODO add inaccessiblememonly effect where `Base.ismutationfree(T)`
@propagate_inbounds Base.getindex(x::ImmutableArray{T}, i::Int) where {T} = x[i]

function Base.setindex!(x::ImmutableArray, val, inds...)
    throw(ArgumentError("Cannot change values of ImmutableArray"))
end

There are some rough edges where Julia relies on similar to produce a new mutable array. StaticArrays.jl is a pretty good reference where you might need to step in and manually manage that.

@chriselrod
Copy link
Contributor

I agree with @MilesCranmer in spirit, but I think half-measures aren't that useful.

C and C++ have const references. They're useful, but far less so than Rust.

Given the declaration:

void foo(std::vector<double> &x, const std::vector<double> &y);

Is y actually const?

No...

void bar(std::vector<double> &x){
  foo(x, x);
}

Thus, just because a function signature says a variable is const&, does not mean that the const& isn't mutating out from under you.
For this reason, the cases are extremely few where a C++ compiler can actually use const to optimize your code.

It is still a useful feature IMO, but it is limited compared to Rust.
In Rust, bar would be illegal: if you have a non-const ref (x), you can't have any other references to it.

This is especially helpful with multi-threading!
But it's useful whenever you have lots of different objects around that you're trying to re-use, such as whenever you're preallocating. You aren't accidentally going to get corrupted state because you ended up using the same object for two different things.
(However, this also makes it hard to represent data structures that don't look like trees in Rust.)

Unlike for C++, where the compilers can do extremely little to optimize const&, in Rust, the compiler knows that no const& objects alias any mut& object, and can thus optimize any associated loads.

While I've complained before about how mutable struct and struct are two different object types, instead of having mutability be an orthogonal part of the type system as it is in Rust and C++, that is another way to guarantee non-aliasing, and why structs in Julia do often optimize well.

So, my point here is that just an immutable wrapper isn't good enough, if it is possible for that memory to alias something that isn't mutable.
ImmutableArray(::Array) should semantically copy the array (ideally with compiler optimizations that can remove the copy if possible), so that it is "impossible" to mutate it and it is unaliased with any mutated Array.

I put impossible in quotes, because presumably we could still pass pointers to C routines like BLAS, and that makes it difficult.
We could require conversion to an Array, but then that'd copy unless the compiler can prove it can elide the copy, which it obviously can't do if we pass a pointer to an opaque ccall; the compiler can't know A and B aren't mutated in mul!(C, A, B)'s gemm.

But I would prefer some heavy handedness -- even forbidding pointer and ccall -- if that would let us guarantee that an ImmutableArray really is not mutating.
I want to look at foo(x::ImmutableArray, args...), and without knowing anything else, be certain that x's contents are not changing. As I already am for anything declared struct in Julia.

@Tokazama
Copy link
Contributor

We definitely need to have a better way of dealing with pointers and private vs public access to struct members. I think there are multiple issues for both of those and I don't recall anyone being against these in the past. So I'm assuming comments here are not arguing against making more progress on those or being dismissive of their importance. They just don't need to be solved entirely before implementing an ImmutableArray type.

Personally, I think the best next step would be defining another GenericMemory alias (e.g., ImmutableMemory{T} = GenericMemory{:const, T, Core.CPU}) that ImmutableArray wraps around. Then we could really optimize functions for ImmutableMemory and just let ImmutableArray inherit the gains (along with other structures that would use ImmutableMemory). Although that still leaves issues with better management of pointers from ImmutableMemory, its not like we aren't using verbose and carefully internalized code for pointers to things like DataType that we assume to never alias.

It would be easier to implement ImmutableArray if we had this all in Julia already, but the absence of these features does not preclude the wrapper array implementation. If people are eager to have ImmutableArray we have the tools to do that right now. We just can't have all the performance optimizations until we finish implementing the back end and might have to change some internal code in the future.

@oscardssmith
Copy link
Member

I don't think we even need ImmutableArray. How often do you want a matrix that you can't mutate? My intuition would be that pretty much every time you want this, it's for the 1d non-resizing case which ImmutableMemory already would handle.

@Tokazama
Copy link
Contributor

I don't think we even need ImmutableArray. How often do you want a matrix that you can't mutate? My intuition would be that pretty much every time you want this, it's for the 1d non-resizing case which ImmutableMemory already would handle.

That's what I'm primarily interested in but I can't speak for others. We might even be able to get away with ImmutableArray{T, N} = ReshapedArray{T,N, ImmutableMemory{T}}. However, that will still take some additional work for methods that rely on similar for initializing the array before computing its elements (map, reduce broadcasting, etc.).

@vtjnash
Copy link
Member

vtjnash commented Mar 26, 2024

# TODO add inaccessiblememonly effect where `Base.ismutationfree(T)`
@propagate_inbounds Base.getindex(x::ImmutableArray{T}, i::Int) where {T} = x[i]

Just a drive-by note that inaccessiblememonly is very much not legal there, since that would imply you are forbidden from constructing this object wrapping any Memory that has been allocated or written to by Julia at any time ever in the past, which is very much useless. Immutable struct objects get around that restriction by writing to the object before it gets returned as an allocated Julia object. You can have a copy-constructor-allocator that does a copy of an existing GenericMemory before the allocation is done and returns that immutable object, but you should not simply mutate Memory and then lie about it afterwards by adding attributes to getindex, unless you enjoy debugging memory corruption and having code that gives wrong answers.

@Tokazama
Copy link
Contributor

Tokazama commented Mar 26, 2024

# TODO add inaccessiblememonly effect where `Base.ismutationfree(T)`
@propagate_inbounds Base.getindex(x::ImmutableArray{T}, i::Int) where {T} = x[i]

Just a drive-by note that inaccessiblememonly is very much not legal there, since that would imply you are forbidden from constructing this object wrapping any Memory that has been allocated or written to by Julia at any time ever in the past, which is very much useless. Immutable struct objects get around that restriction by writing to the object before it gets returned as an allocated Julia object. You can have a copy-constructor-allocator that does a copy of an existing GenericMemory before the allocation is done and returns that immutable object, but you should not simply mutate Memory and then lie about it afterwards by adding attributes to getindex, unless you enjoy debugging memory corruption and having code that gives wrong answers.

I agree that we need to figure other things out before asserting effects, which is why I didn't actually implement it. I just included that note to give an idea of what kind of optimizations we might be implementing later on.

@vtjnash
Copy link
Member

vtjnash commented Mar 26, 2024

Yes, that is why I am pointing out that no additional optimizations are valid to implement on that later on.

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.