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

Add _unsetindex! methods for SubArrays and CartesianIndexes #53383

Merged
merged 3 commits into from
Feb 20, 2024

Conversation

jishnub
Copy link
Contributor

@jishnub jishnub commented Feb 19, 2024

With this, the following (and equivalent calls) work:

julia> copyto!(view(zeros(BigInt, 2), 1:2), Vector{BigInt}(undef,2))
2-element view(::Vector{BigInt}, 1:2) with eltype BigInt:
 #undef
 #undef

julia> copyto!(view(zeros(BigInt, 2), 1:2), view(Vector{BigInt}(undef,2), 1:2))
2-element view(::Vector{BigInt}, 1:2) with eltype BigInt:
 #undef
 #undef

Close #53098. With this, all the _unsetindex! branches in copyto_unaliased! work for Array-views, and this makes certain indexing operations vectorize and speed-up:

julia> using BenchmarkTools

julia> a = view(rand(100,100), 1:100, 1:100); b = view(similar(a), axes(a)...);

julia> @btime copyto!($b, $a);
  16.427 μs (0 allocations: 0 bytes) # master
  2.308 μs (0 allocations: 0 bytes) # PR

Improves (but doesn't resolve) #40962 and #53158

julia> a = rand(40,40); b = rand(40,40);

julia> @btime $a[1:end,1:end] .= $b;
  5.383 μs (0 allocations: 0 bytes) # v"1.12.0-DEV.16"
  3.194 μs (0 allocations: 0 bytes) # PR

@jishnub jishnub added performance Must go faster arrays [a, r, r, a, y, s] labels Feb 19, 2024
@jishnub jishnub requested review from Seelengrab and oscardssmith and removed request for Seelengrab February 19, 2024 08:01
@vtjnash vtjnash merged commit 1a90409 into master Feb 20, 2024
7 checks passed
@vtjnash vtjnash deleted the jishnub/unsetindex branch February 20, 2024 17:05
@jishnub jishnub added backport 1.11 Change should be backported to release-1.11 and removed backport 1.11 Change should be backported to release-1.11 labels Feb 20, 2024
tecosaur pushed a commit to tecosaur/julia that referenced this pull request Mar 4, 2024
…liaLang#53383)

With this, the following (and equivalent calls) work:
```julia
julia> copyto!(view(zeros(BigInt, 2), 1:2), Vector{BigInt}(undef,2))
2-element view(::Vector{BigInt}, 1:2) with eltype BigInt:
 #undef
 #undef

julia> copyto!(view(zeros(BigInt, 2), 1:2), view(Vector{BigInt}(undef,2), 1:2))
2-element view(::Vector{BigInt}, 1:2) with eltype BigInt:
 #undef
 #undef
```

Close JuliaLang#53098. With this, all
the `_unsetindex!` branches in `copyto_unaliased!` work for
`Array`-views, and this makes certain indexing operations vectorize and
speed-up:
```julia
julia> using BenchmarkTools

julia> a = view(rand(100,100), 1:100, 1:100); b = view(similar(a), axes(a)...);

julia> @Btime copyto!($b, $a);
  16.427 μs (0 allocations: 0 bytes) # master
  2.308 μs (0 allocations: 0 bytes) # PR
``` 

Improves (but doesn't resolve)
JuliaLang#40962 and
JuliaLang#53158

```julia
julia> a = rand(40,40); b = rand(40,40);

julia> @Btime $a[1:end,1:end] .= $b;
  5.383 μs (0 allocations: 0 bytes) # v"1.12.0-DEV.16"
  3.194 μs (0 allocations: 0 bytes) # PR
```
ƒ
Co-authored-by: Jameson Nash <[email protected]>
mkitti pushed a commit to mkitti/julia that referenced this pull request Mar 7, 2024
…liaLang#53383)

With this, the following (and equivalent calls) work:
```julia
julia> copyto!(view(zeros(BigInt, 2), 1:2), Vector{BigInt}(undef,2))
2-element view(::Vector{BigInt}, 1:2) with eltype BigInt:
 #undef
 #undef

julia> copyto!(view(zeros(BigInt, 2), 1:2), view(Vector{BigInt}(undef,2), 1:2))
2-element view(::Vector{BigInt}, 1:2) with eltype BigInt:
 #undef
 #undef
```

Close JuliaLang#53098. With this, all
the `_unsetindex!` branches in `copyto_unaliased!` work for
`Array`-views, and this makes certain indexing operations vectorize and
speed-up:
```julia
julia> using BenchmarkTools

julia> a = view(rand(100,100), 1:100, 1:100); b = view(similar(a), axes(a)...);

julia> @Btime copyto!($b, $a);
  16.427 μs (0 allocations: 0 bytes) # master
  2.308 μs (0 allocations: 0 bytes) # PR
``` 

Improves (but doesn't resolve)
JuliaLang#40962 and
JuliaLang#53158

```julia
julia> a = rand(40,40); b = rand(40,40);

julia> @Btime $a[1:end,1:end] .= $b;
  5.383 μs (0 allocations: 0 bytes) # v"1.12.0-DEV.16"
  3.194 μs (0 allocations: 0 bytes) # PR
```
ƒ
Co-authored-by: Jameson Nash <[email protected]>
@jishnub jishnub added the backport 1.11 Change should be backported to release-1.11 label Mar 17, 2024
@KristofferC KristofferC mentioned this pull request Mar 20, 2024
41 tasks
KristofferC pushed a commit that referenced this pull request Mar 27, 2024
…3383)

With this, the following (and equivalent calls) work:
```julia
julia> copyto!(view(zeros(BigInt, 2), 1:2), Vector{BigInt}(undef,2))
2-element view(::Vector{BigInt}, 1:2) with eltype BigInt:
 #undef
 #undef

julia> copyto!(view(zeros(BigInt, 2), 1:2), view(Vector{BigInt}(undef,2), 1:2))
2-element view(::Vector{BigInt}, 1:2) with eltype BigInt:
 #undef
 #undef
```

Close #53098. With this, all
the `_unsetindex!` branches in `copyto_unaliased!` work for
`Array`-views, and this makes certain indexing operations vectorize and
speed-up:
```julia
julia> using BenchmarkTools

julia> a = view(rand(100,100), 1:100, 1:100); b = view(similar(a), axes(a)...);

julia> @Btime copyto!($b, $a);
  16.427 μs (0 allocations: 0 bytes) # master
  2.308 μs (0 allocations: 0 bytes) # PR
```

Improves (but doesn't resolve)
#40962 and
#53158

```julia
julia> a = rand(40,40); b = rand(40,40);

julia> @Btime $a[1:end,1:end] .= $b;
  5.383 μs (0 allocations: 0 bytes) # v"1.12.0-DEV.16"
  3.194 μs (0 allocations: 0 bytes) # PR
```
ƒ
Co-authored-by: Jameson Nash <[email protected]>

(cherry picked from commit 1a90409)
KristofferC added a commit that referenced this pull request Apr 9, 2024
Backported PRs:
- [x] #53757 <!-- Add an IndexStyle example to the diagind docstring -->
- [x] #53809 <!-- Add missing GC_POP() in emit_cfunction -->
- [x] #53789 <!-- also check that UUID of project is non-null when
treating it as a package -->
- [x] #53805 <!-- precompilepkgs: simplify custom config printing if
only one -->
- [x] #53822 <!-- Bump libuv -->
- [x] #53837 <!-- update MPFR to 4.2.1 -->
- [x] #53862 <!-- precompilepkgs: fix error reporting -->
- [x] #53774 <!-- Remove some duplicates from emitted compilation traces
-->
- [ ] #53696 <!-- add invokelatest to on_done callback in bracketed
paste -->
- [x] #53383 <!-- Add `_unsetindex!` methods for `SubArray`s and
`CartesianIndex`es -->
- [x] #53475 <!-- Fix boundscheck in unsetindex for SubArrays -->
- [x] #53888 
- [x] #53870 <!-- Revert change to checksum for llvm-julia -->
- [x] #53906 <!-- Add `Base.isrelocatable(pkg)` -->
- [x] #53833 <!-- Profile: make heap snapshots viewable in vscode viewer
-->
- [x] #53961 <!-- `LazyString` in `LinearAlgebra.checksquare` error
message -->
- [x] #53962 <!-- Use StringMemory instead of StringVector where
possible -->
- [x] #53825 <!-- profile: doc: update the `Allocs.@profile` doc string
-->
- [x] #53975 <!-- `LazyString` in `DimensionMismatch` error messages in
broadcasting -->
- [x] #53905 <!-- Avoid repeated precompilation when loading from
non-relocatable cachefiles -->
- [x] #53896 <!-- Make reshape and view on Memory produce Arrays and
delete wrap -->
- [x] #53991 <!-- Test and fix non-int-length bug in `view(::Memory,
::Union{UnitRange, Base.OneTo})` -->
@KristofferC KristofferC removed the backport 1.11 Change should be backported to release-1.11 label Apr 17, 2024
KristofferC added a commit that referenced this pull request May 6, 2024
KristofferC added a commit that referenced this pull request May 13, 2024
KristofferC added a commit that referenced this pull request May 13, 2024
…x`es (#53383)"

This reverts commit 1a90409.

(cherry picked from commit f8a7496)
KristofferC added a commit that referenced this pull request May 23, 2024
…x`es (#53383)"

This reverts commit 1a90409.

(cherry picked from commit f8a7496)
lazarusA pushed a commit to lazarusA/julia that referenced this pull request Jul 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arrays [a, r, r, a, y, s] performance Must go faster
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The IndexCartesian branch of copyto! is broken as _unsetindex is undefined for CartesianIndex arguments
4 participants