Skip to content

Default to SharedStorage#822

Open
christiangnrd wants to merge 7 commits into
mainfrom
shared
Open

Default to SharedStorage#822
christiangnrd wants to merge 7 commits into
mainfrom
shared

Conversation

@christiangnrd

Copy link
Copy Markdown
Member

No description provided.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Metal Benchmarks

Details
Benchmark suite Current: 4492e5f Previous: b8d46ce Ratio
array/accumulate/Float32/1d 578125 ns 515334 ns 1.12
array/accumulate/Float32/dims=1 578750 ns 506708 ns 1.14
array/accumulate/Float32/dims=1L 8865292 ns 8950000 ns 0.99
array/accumulate/Float32/dims=2 628333 ns 538958 ns 1.17
array/accumulate/Float32/dims=2L 3368312.5 ns 2913312.5 ns 1.16
array/accumulate/Int64/1d 905958 ns 931417 ns 0.97
array/accumulate/Int64/dims=1 1110354.5 ns 1114292 ns 1.00
array/accumulate/Int64/dims=1L 9592084 ns 9781937 ns 0.98
array/accumulate/Int64/dims=2 1471791 ns 1454062.5 ns 1.01
array/accumulate/Int64/dims=2L 6608875 ns 6944084 ns 0.95
array/broadcast 376750 ns 333167 ns 1.13
array/construct 3375 ns 3333 ns 1.01
array/permutedims/2d 639333.5 ns 619917 ns 1.03
array/permutedims/3d 1101167 ns 1099417 ns 1.00
array/permutedims/4d 1710750.5 ns 1231833 ns 1.39
array/private/copy 401625 ns 393104 ns 1.02
array/private/copyto!/cpu_to_gpu 381270.5 ns 368604.5 ns 1.03
array/private/copyto!/gpu_to_cpu 382917 ns 357542 ns 1.07
array/private/copyto!/gpu_to_gpu 359667 ns 325791 ns 1.10
array/private/iteration/findall/bool 1147375 ns 1192542 ns 0.96
array/private/iteration/findall/int 1287167 ns 1323145.5 ns 0.97
array/private/iteration/findfirst/bool 1402395.5 ns 1315083 ns 1.07
array/private/iteration/findfirst/int 1466167 ns 1375917 ns 1.07
array/private/iteration/findmin/1d 1592416 ns 1520792 ns 1.05
array/private/iteration/findmin/2d 1275979 ns 1229812.5 ns 1.04
array/private/iteration/logical 1916458 ns 1990958 ns 0.96
array/private/iteration/scalar 2854084 ns 1747333 ns 1.63
array/random/rand/Float32 616416 ns 603209 ns 1.02
array/random/rand/Int64 636291 ns 641812.5 ns 0.99
array/random/rand!/Float32 528208 ns 498917 ns 1.06
array/random/rand!/Int64 495458.5 ns 466875 ns 1.06
array/random/randn/Float32 578917 ns 561042 ns 1.03
array/random/randn!/Float32 481334 ns 454937.5 ns 1.06
array/reductions/mapreduce/Float32/1d 435167 ns 620042 ns 0.70
array/reductions/mapreduce/Float32/dims=1 483458 ns 442167 ns 1.09
array/reductions/mapreduce/Float32/dims=1L 691958 ns 701292 ns 0.99
array/reductions/mapreduce/Float32/dims=2 501375 ns 449542 ns 1.12
array/reductions/mapreduce/Float32/dims=2L 1347750 ns 1001125 ns 1.35
array/reductions/mapreduce/Int64/1d 639000 ns 808375 ns 0.79
array/reductions/mapreduce/Int64/dims=1 829458 ns 758375 ns 1.09
array/reductions/mapreduce/Int64/dims=1L 1081583 ns 1102250 ns 0.98
array/reductions/mapreduce/Int64/dims=2 984292 ns 812583 ns 1.21
array/reductions/mapreduce/Int64/dims=2L 2221292 ns 2213417 ns 1.00
array/reductions/reduce/Float32/1d 414354.5 ns 610625 ns 0.68
array/reductions/reduce/Float32/dims=1 491792 ns 446333.5 ns 1.10
array/reductions/reduce/Float32/dims=1L 695250 ns 702500 ns 0.99
array/reductions/reduce/Float32/dims=2 405375 ns 356917 ns 1.14
array/reductions/reduce/Float32/dims=2L 494709 ns 497708 ns 0.99
array/reductions/reduce/Int64/1d 644145.5 ns 798708 ns 0.81
array/reductions/reduce/Int64/dims=1 821000 ns 760000 ns 1.08
array/reductions/reduce/Int64/dims=1L 1075917 ns 1097542 ns 0.98
array/reductions/reduce/Int64/dims=2 319750 ns 293833.5 ns 1.09
array/reductions/reduce/Int64/dims=2L 687375 ns 689500 ns 1.00
array/shared/copy 154542 ns 158167 ns 0.98
array/shared/copyto!/cpu_to_gpu 39291 ns 39584 ns 0.99
array/shared/copyto!/gpu_to_cpu 40417 ns 39917 ns 1.01
array/shared/copyto!/gpu_to_gpu 41000 ns 40292 ns 1.02
array/shared/iteration/findall/bool 1155166 ns 1199792 ns 0.96
array/shared/iteration/findall/int 1299270.5 ns 1330625 ns 0.98
array/shared/iteration/findfirst/bool 1122667 ns 994708 ns 1.13
array/shared/iteration/findfirst/int 1209583 ns 1020625 ns 1.19
array/shared/iteration/findmin/1d 1322667 ns 1281750 ns 1.03
array/shared/iteration/findmin/2d 1277500 ns 1231125 ns 1.04
array/shared/iteration/logical 1721229.5 ns 1796688 ns 0.96
array/shared/iteration/scalar 3604.125 ns 3437.5 ns 1.05
array/sorting/1d 1751375 ns 1993833.5 ns 0.88
array/sorting/2d 8203208 ns 8450062.5 ns 0.97
integration/byval/reference 1139208 ns 1150062.5 ns 0.99
integration/byval/slices=1 1138167 ns 1151750 ns 0.99
integration/byval/slices=2 2050166 ns 2079167 ns 0.99
integration/byval/slices=3 8199666 ns 18009271 ns 0.46
integration/metaldevrt 449459 ns 439250 ns 1.02
kernel/indexing 369395.5 ns 331500 ns 1.11
kernel/indexing_checked 541167 ns 492500 ns 1.10
kernel/launch 1975 ns 1954.1 ns 1.01
kernel/rand 523000 ns 497209 ns 1.05
latency/import 1789909209 ns 1812323292 ns 0.99
latency/precompile 39262096271 ns 39925435375 ns 0.98
latency/ttfp 2134295417 ns 2150481396 ns 0.99
metal/synchronization/context 559.7460317460317 ns 534.3264248704663 ns 1.05
metal/synchronization/stream 287.2 ns 274.8666666666667 ns 1.04

This comment was automatically generated by workflow using github-action-benchmark.

Comment thread src/array.jl Outdated
@codecov

codecov Bot commented Jun 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 42.85714% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.08%. Comparing base (b8d46ce) to head (4492e5f).

Files with missing lines Patch % Lines
src/Metal.jl 0.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #822      +/-   ##
==========================================
- Coverage   86.62%   86.08%   -0.54%     
==========================================
  Files          76       76              
  Lines        5144     5147       +3     
==========================================
- Hits         4456     4431      -25     
- Misses        688      716      +28     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@christiangnrd christiangnrd force-pushed the shared branch 3 times, most recently from fdf7a32 to 97ae406 Compare June 18, 2026 12:36
@christiangnrd

Copy link
Copy Markdown
Member Author

Could the github actions failure be related to the compilation failures?

@maleadt

maleadt commented Jun 19, 2026

Copy link
Copy Markdown
Member

I'm not sure how. There's a known issue with ObjC errors leaking outside of their retain/release scope and crashing during error reporting, but that's read-only and shouldn't cause a crash in LLVM.

@maleadt

maleadt commented Jun 19, 2026

Copy link
Copy Markdown
Member

I guess the remaining question is semantics. Do we want to allow scalar iteration on shard memory so that it can be used with CPU code? It'll never be as fast as Array, but if we port the dirty memory flag tracking from CUDA.jl we can get it down to a couple of ns (as opposed to ~1ns for an Array getindex or so). If we want to add back scalar iteration checking we add another couple of ns for the TLS check on every access.

@christiangnrd

Copy link
Copy Markdown
Member Author

i.e. unsafe_wrap(Array, ... wouldn't be needed?

@maleadt

maleadt commented Jun 19, 2026

Copy link
Copy Markdown
Member

Right, that's the current design of CUDA.jl:

Precompiling CUDA finished.
  12 dependencies successfully precompiled in 43 seconds. 90 already precompiled.

julia> CUDA.allowscalar(false)

julia> a = cu([1])
1-element CuArray{Int64, 1, CUDACore.DeviceMemory}:
 1

julia> a[]
ERROR: Scalar indexing is disallowed.

julia> b = cu([1]; unified=true)
1-element CuArray{Int64, 1, CUDACore.UnifiedMemory}:
 1

julia> b[]
1

I'm not entirely convinced this is the best option though. It makes sense, but people often use allowscalar for detecting GPU code. We could tell them they have to use private memory for that, but it still makes allowscalar(false) a lie.

@christiangnrd

Copy link
Copy Markdown
Member Author

This is also how shared storage currently works

@christiangnrd

Copy link
Copy Markdown
Member Author

But without the dirty flag so there might be some latent race conditions with shared MtlArrays

@maleadt

maleadt commented Jun 19, 2026

Copy link
Copy Markdown
Member

This is also how shared storage currently works

Right, but it never was the default. It would break users doing allowscalar(false) to detect GPU functionality execution on the CPU.

And we definitely need the dirty flag to improve performance here. But that can be follow-up work.

@christiangnrd

Copy link
Copy Markdown
Member Author

What of we add a @warn when they call allowscalar the first time if default storage mode is shared?

@maleadt

maleadt commented Jun 19, 2026

Copy link
Copy Markdown
Member

I guess that could work, but the interface is not extensible like that right now.

EDIT: I guess we could implement Metal.allowscalar, have it warn, and then call GPUArrays.allowscalar.

@christiangnrd

Copy link
Copy Markdown
Member Author

New relevant case: unsupported sorts (by != identity) will silently run on the CPU under SharedStorage.

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.

2 participants