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

Improve performance of column operators #829

Merged
merged 1 commit into from
Jul 27, 2022
Merged

Improve performance of column operators #829

merged 1 commit into from
Jul 27, 2022

Conversation

charleskawczynski
Copy link
Member

This PR improves the performance of column operations by:

  • Adding @inline and @inbounds annotations
  • Fixing some type inference issues that prevented inlining (in idxin)

src/DataLayouts/struct.jl Outdated Show resolved Hide resolved
src/Fields/indices.jl Outdated Show resolved Hide resolved
src/Fields/indices.jl Outdated Show resolved Hide resolved
src/Geometry/axistensors.jl Outdated Show resolved Hide resolved
src/Geometry/axistensors.jl Outdated Show resolved Hide resolved
src/Operators/finitedifference.jl Outdated Show resolved Hide resolved
src/Operators/finitedifference.jl Outdated Show resolved Hide resolved
src/Operators/finitedifference.jl Outdated Show resolved Hide resolved
src/Operators/finitedifference.jl Outdated Show resolved Hide resolved
src/Operators/finitedifference.jl Outdated Show resolved Hide resolved
@charleskawczynski charleskawczynski force-pushed the ck/fix_col_perf branch 3 times, most recently from a2e7adf to e8f8e13 Compare July 26, 2022 03:32
@charleskawczynski
Copy link
Member Author

bors try

bors bot added a commit that referenced this pull request Jul 26, 2022
@bors
Copy link
Contributor

bors bot commented Jul 26, 2022

try

Build failed:

@charleskawczynski
Copy link
Member Author

bors r+

bors bot added a commit that referenced this pull request Jul 26, 2022
829: Improve performance of column operators r=charleskawczynski a=charleskawczynski

This PR improves the performance of column operations by:
 - Adding ``@inline`` and ``@inbounds`` annotations
 - Fixing some type inference issues that prevented inlining (in `idxin`)

Co-authored-by: Charles Kawczynski <kawczynski.charles@gmail.com>
@bors
Copy link
Contributor

bors bot commented Jul 26, 2022

Build failed:

@charleskawczynski
Copy link
Member Author

bors r+

bors bot added a commit that referenced this pull request Jul 26, 2022
829: Improve performance of column operators r=charleskawczynski a=charleskawczynski

This PR improves the performance of column operations by:
 - Adding ``@inline`` and ``@inbounds`` annotations
 - Fixing some type inference issues that prevented inlining (in `idxin`)

Co-authored-by: Charles Kawczynski <kawczynski.charles@gmail.com>
@bors
Copy link
Contributor

bors bot commented Jul 26, 2022

Canceled.

@charleskawczynski
Copy link
Member Author

bors r+

bors bot added a commit that referenced this pull request Jul 26, 2022
829: Improve performance of column operators r=charleskawczynski a=charleskawczynski

This PR improves the performance of column operations by:
 - Adding ``@inline`` and ``@inbounds`` annotations
 - Fixing some type inference issues that prevented inlining (in `idxin`)

Co-authored-by: Charles Kawczynski <kawczynski.charles@gmail.com>
@bors
Copy link
Contributor

bors bot commented Jul 26, 2022

Build failed:

@charleskawczynski
Copy link
Member Author

bors r+

bors bot added a commit that referenced this pull request Jul 26, 2022
829: Improve performance of column operators r=charleskawczynski a=charleskawczynski

This PR improves the performance of column operations by:
 - Adding ``@inline`` and ``@inbounds`` annotations
 - Fixing some type inference issues that prevented inlining (in `idxin`)

Co-authored-by: Charles Kawczynski <kawczynski.charles@gmail.com>
@bors
Copy link
Contributor

bors bot commented Jul 26, 2022

Build failed:

@charleskawczynski
Copy link
Member Author

bors try

bors bot added a commit that referenced this pull request Jul 27, 2022
@charleskawczynski
Copy link
Member Author

bors r+

bors bot added a commit that referenced this pull request Jul 27, 2022
829: Improve performance of column operators r=charleskawczynski a=charleskawczynski

This PR improves the performance of column operations by:
 - Adding ``@inline`` and ``@inbounds`` annotations
 - Fixing some type inference issues that prevented inlining (in `idxin`)

Co-authored-by: Charles Kawczynski <kawczynski.charles@gmail.com>
@bors
Copy link
Contributor

bors bot commented Jul 27, 2022

Build failed:

@charleskawczynski
Copy link
Member Author

bors r+

bors bot added a commit that referenced this pull request Jul 27, 2022
829: Improve performance of column operators r=charleskawczynski a=charleskawczynski

This PR improves the performance of column operations by:
 - Adding ``@inline`` and ``@inbounds`` annotations
 - Fixing some type inference issues that prevented inlining (in `idxin`)

Co-authored-by: Charles Kawczynski <kawczynski.charles@gmail.com>
@bors
Copy link
Contributor

bors bot commented Jul 27, 2022

Build failed:

@charleskawczynski
Copy link
Member Author

bors r+

bors bot added a commit that referenced this pull request Jul 27, 2022
829: Improve performance of column operators r=charleskawczynski a=charleskawczynski

This PR improves the performance of column operations by:
 - Adding ``@inline`` and ``@inbounds`` annotations
 - Fixing some type inference issues that prevented inlining (in `idxin`)

Co-authored-by: Charles Kawczynski <kawczynski.charles@gmail.com>
@bors
Copy link
Contributor

bors bot commented Jul 27, 2022

Build failed:

Add @inbounds inside getidx

Add @inline to some getidx and setidx

Add @inline annot to getidx

Add @inline annotations, some tests are fixed

Nearly fix tests with unrolling idxin

Fix merge conflict

Add another example in comments

Make tests less flakey, higher res

Remove temporary main timings

Apply formatter

Fix JET failure

Fix more tests

More inline and fix tests

More inlining

Clean up

Temporarily make bors not depend on windows ci

Increase benchmark buffer

Disable fail fast in GHA while windows is broken

Try to do the right thing
@charleskawczynski
Copy link
Member Author

bors r+

@bors bors bot merged commit efd1bf6 into main Jul 27, 2022
@bors bors bot deleted the ck/fix_col_perf branch July 27, 2022 07:07
bors bot added a commit that referenced this pull request Aug 26, 2022
917: Do inlining and inbounds more carefully r=charleskawczynski a=charleskawczynski

This PR is an attempt to partially revert (and complete) #829, by:
 - Exchanging the use of ``@inline`` and ``@inbounds``
 - Removing ``@inline`` for "expensive" functions that do not have or uses indices which require boundscheck elision
 - Applying inlining and ``@inbounds`` to slab functions

The only thing that I'm not 100% sure about is the ``@inbounds`` around the threading loops, does what I've done look okay? cc `@simonbyrne` 

Supersedes #891.

Co-authored-by: Charles Kawczynski <kawczynski.charles@gmail.com>
bors bot added a commit that referenced this pull request Aug 26, 2022
917: Do inlining and inbounds more carefully r=charleskawczynski a=charleskawczynski

This PR is an attempt to partially revert (and extend/complete) #829, by:
 - More properly using ``@inline`,` ``@inbounds`` and `Base.`@propagate_inbounds``
 - Removing ``@inline`` for "expensive" functions that do not have or uses indices which require boundscheck elision
 - Applying inlining and ``@inbounds`` to slab functions

The only thing that I'm not 100% sure about is the ``@inbounds`` around the threading loops, does what I've done look okay? cc `@simonbyrne` 

Supersedes #891.

Co-authored-by: Charles Kawczynski <kawczynski.charles@gmail.com>
bors bot added a commit that referenced this pull request Aug 26, 2022
917: Do inlining and inbounds more carefully r=charleskawczynski a=charleskawczynski

This PR is an attempt to partially revert (and extend/complete) #829, by:
 - More properly using ``@inline`,` ``@inbounds`` and `Base.`@propagate_inbounds``
 - Removing ``@inline`` for "expensive" functions that do not have or uses indices which require boundscheck elision
 - Applying inlining and ``@inbounds`` to slab functions

The only thing that I'm not 100% sure about is the ``@inbounds`` around the threading loops, does what I've done look okay? cc `@simonbyrne` 

Supersedes #891.

Co-authored-by: Charles Kawczynski <kawczynski.charles@gmail.com>
bors bot added a commit that referenced this pull request Aug 26, 2022
917: Do inlining and inbounds more carefully r=charleskawczynski a=charleskawczynski

This PR is an attempt to partially revert (and extend/complete) #829, by:
 - More properly using ``@inline`,` ``@inbounds`` and `Base.`@propagate_inbounds``
 - Removing ``@inline`` for "expensive" functions that do not have or uses indices which require boundscheck elision
 - Applying inlining and ``@inbounds`` to slab functions

The only thing that I'm not 100% sure about is the ``@inbounds`` around the threading loops, does what I've done look okay? cc `@simonbyrne` 

Supersedes #891.

Co-authored-by: Charles Kawczynski <kawczynski.charles@gmail.com>
bors bot added a commit that referenced this pull request Aug 27, 2022
917: Do inlining and inbounds more carefully r=charleskawczynski a=charleskawczynski

This PR is an attempt to partially revert (and extend/complete) #829, by:
 - More properly using ``@inline`,` ``@inbounds`` and `Base.`@propagate_inbounds``
 - Removing ``@inline`` for "expensive" functions that do not have or uses indices which require boundscheck elision
 - Applying inlining and ``@inbounds`` to slab functions

The only thing that I'm not 100% sure about is the ``@inbounds`` around the threading loops, does what I've done look okay? cc `@simonbyrne` 

Supersedes #891.

Co-authored-by: Charles Kawczynski <kawczynski.charles@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant