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.

Sign up for GitHub

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

Jump to bottom

Fix performance regression in broadcasting with CartesianIndices #39333

Merged
merged 2 commits into from
Jan 21, 2021
Merged

Fix performance regression in broadcasting with CartesianIndices #39333

merged 2 commits into from
Jan 21, 2021

Conversation

kimikage
Copy link
Contributor

@kimikage kimikage commented Jan 20, 2021 •
edited
Loading

This avoids the boundary check due to a change in the implementation of iteration using CartesianIndices in PR #37829.

This is a workaround on the caller side and does not change the iteration mechanism itself.

julia> xu8 = rand(UInt8, 1000, 1000); yu8 = rand(UInt8, 1000, 1000);

julia> @btime $xu8 .+ $yu8;
  795.800 μs (2 allocations: 976.70 KiB) # before (master and v1.6.0-beta1)
  85.900 μs (2 allocations: 976.70 KiB) # after

Thanks to @chriselrod.

Fixes #38086

@kimikage
Fix performance regression in broadcasting with CartesianIndices
398c2ab 
This avoids the boundary check due to a change in the implementation
of iteration using `CartecianIndices` in PR JuliaLang#37829.
This is a workaround on the caller side and does not change
the iteration mechanism itself.
@kimikage kimikage closed this Jan 20, 2021
@kimikage kimikage reopened this Jan 20, 2021
@kimikage
Copy link
Contributor Author

Have I been disrespectful to buildbot in any way?

@KristofferC
Copy link
Sponsor Member

Sometimes you have to wait a bit for the builds to start.

@mbauman mbauman added backport 1.6 Change should be backported to release-1.6 broadcast Applying a function over a collection performance Must go faster labels Jan 20, 2021
thofma
thofma reviewed Jan 20, 2021
View reviewed changes
base/broadcast.jl Outdated Show resolved Hide resolved
@KristofferC KristofferC mentioned this pull request Jan 20, 2021
Merged
60 tasks
@KristofferC KristofferC merged commit a4cd68c into JuliaLang:master Jan 21, 2021
KristofferC pushed a commit that referenced this pull request Jan 21, 2021
@kimikage @KristofferC
Fix performance regression in broadcasting with CartesianIndices (#39333
f28385c 
)

* Fix performance regression in broadcasting with CartesianIndices

This avoids the boundary check due to a change in the implementation
of iteration using `CartecianIndices` in PR #37829.
This is a workaround on the caller side and does not change
the iteration mechanism itself.

Co-authored-by: Matt Bauman <[email protected]>
Co-authored-by: thofma <[email protected]>
(cherry picked from commit a4cd68c)
@kimikage kimikage deleted the issue38086 branch January 21, 2021 11:21
@KristofferC KristofferC removed the backport 1.6 Change should be backported to release-1.6 label Feb 1, 2021
KristofferC pushed a commit that referenced this pull request Feb 1, 2021
@kimikage @KristofferC
Fix performance regression in broadcasting with CartesianIndices (#39333
cf0c32f 
)

* Fix performance regression in broadcasting with CartesianIndices

This avoids the boundary check due to a change in the implementation
of iteration using `CartecianIndices` in PR #37829.
This is a workaround on the caller side and does not change
the iteration mechanism itself.

Co-authored-by: Matt Bauman <[email protected]>
Co-authored-by: thofma <[email protected]>
(cherry picked from commit a4cd68c)
@KristofferC KristofferC mentioned this pull request Mar 29, 2021
Open
ElOceanografo pushed a commit to ElOceanografo/julia that referenced this pull request May 4, 2021
@kimikage @mbauman
@thofma @ElOceanografo
Fix performance regression in broadcasting with CartesianIndices (Jul…
af5aff2 
…iaLang#39333)

* Fix performance regression in broadcasting with CartesianIndices

This avoids the boundary check due to a change in the implementation
of iteration using `CartecianIndices` in PR JuliaLang#37829.
This is a workaround on the caller side and does not change
the iteration mechanism itself.


Co-authored-by: Matt Bauman <[email protected]>
Co-authored-by: thofma <[email protected]>
antoine-levitt pushed a commit to antoine-levitt/julia that referenced this pull request May 9, 2021
@kimikage @mbauman
@thofma @antoine-levitt
Fix performance regression in broadcasting with CartesianIndices (Jul…
3e6e276 
…iaLang#39333)

* Fix performance regression in broadcasting with CartesianIndices

This avoids the boundary check due to a change in the implementation
of iteration using `CartecianIndices` in PR JuliaLang#37829.
This is a workaround on the caller side and does not change
the iteration mechanism itself.


Co-authored-by: Matt Bauman <[email protected]>
Co-authored-by: thofma <[email protected]>
staticfloat pushed a commit that referenced this pull request Dec 23, 2022
@kimikage @staticfloat
Fix performance regression in broadcasting with CartesianIndices (#39333
478790e 
)

* Fix performance regression in broadcasting with CartesianIndices

This avoids the boundary check due to a change in the implementation
of iteration using `CartecianIndices` in PR #37829.
This is a workaround on the caller side and does not change
the iteration mechanism itself.

Co-authored-by: Matt Bauman <[email protected]>
Co-authored-by: thofma <[email protected]>
(cherry picked from commit a4cd68c)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thofma thofma thofma left review comments

@mbauman mbauman mbauman approved these changes

Assignees
No one assigned
Labels
broadcast Applying a function over a collection performance Must go faster
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Performance regression in broadcasting with CartesianIndices on v1.6.0-DEV
4 participants
@kimikage @KristofferC @mbauman @thofma