Skip to content

Conversation

@jishnub
Copy link
Member

@jishnub jishnub commented Apr 10, 2025

This reduces TTFX as well as improves performance. This is because we add specialized functions to avoid the branches in getindex for a triangular matrix.

julia> using LinearAlgebra

julia> U4 = UpperTriangular(ones(4,4));

julia> @time Array(U4); # TTFX
  0.087534 seconds (193.27 k allocations: 9.706 MiB, 99.88% compilation time) # nightly
  0.049869 seconds (91.45 k allocations: 4.531 MiB, 99.78% compilation time) # this PR

julia> U = UpperTriangular(ones(2000,2000));

julia> @btime Array($U);
  3.205 ms (3 allocations: 30.52 MiB) # master
  2.764 ms (3 allocations: 30.52 MiB) # this PR

@jishnub jishnub added arrays [a, r, r, a, y, s] ttfx The change pertains to first-call latency labels Apr 10, 2025
@dkarrasch
Copy link
Member

What's the rationale here?

@jishnub jishnub added the performance Must go faster label Apr 10, 2025
@jishnub
Copy link
Member Author

jishnub commented Apr 10, 2025

The idea here is that we remove the branches in getindex, and move the conditions to the loop ranges instead (we loop over the stored and non-stored rows separately). We also hardcode the zero element corresponding to the non-stored indices instead of relying on getindex, whereas for the stored indices, we forward the indexing to the parent. This makes the copy simpler to compile, as well as faster to execute.

@codecov
Copy link

codecov bot commented Apr 10, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.04%. Comparing base (830ea2f) to head (bef96b4).
Report is 7 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1282   +/-   ##
=======================================
  Coverage   92.03%   92.04%           
=======================================
  Files          34       34           
  Lines       15499    15515   +16     
=======================================
+ Hits        14265    14281   +16     
  Misses       1234     1234           

☔ View full report in Codecov by Sentry.
📢 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.

@jishnub jishnub force-pushed the jishnub/tri_matrix branch 2 times, most recently from f739ba7 to a2e74a4 Compare April 13, 2025 11:25
@jishnub jishnub force-pushed the jishnub/tri_matrix branch from bb157ba to bef96b4 Compare April 14, 2025 17:38
@jishnub
Copy link
Member Author

jishnub commented Apr 21, 2025

Should we go ahead with this?

@jishnub jishnub merged commit d21ad8c into master Apr 21, 2025
4 checks passed
@jishnub jishnub deleted the jishnub/tri_matrix branch April 21, 2025 18:20
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 ttfx The change pertains to first-call latency

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants