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

Delete redundant xcat methods that previously improved performance #54477

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

LilithHafner
Copy link
Member

Deleting these because I ran ./julia --startup-file=no --inline=no -e '@time Base.runtests("ranges")' before and after this PR and found a 2 second (1%) regression, which is within margin of error of zero and way less than the original performance improvement when these were added in 2015 in #11158 by @timholy

@LilithHafner LilithHafner added the kind:excision Removal of code from Base or the repository label May 15, 2024
@vtjnash
Copy link
Sponsor Member

vtjnash commented May 15, 2024

We might want to run PkgEval to see if any packages will need to be fixed also. Or at least check on the next nightly run if any package test was relying on this

@KristofferC KristofferC added needs pkgeval Tests for all registered packages should be run with this change kind:excision Removal of code from Base or the repository and removed kind:excision Removal of code from Base or the repository labels May 15, 2024
@LilithHafner
Copy link
Member Author

The vcat ambiguity situation is a trashfire. I thought this standalone PR might help, but it causes ambiguities. I'll need to combine it with reducing piracy. As is this creates an ambiguity with LinearAlgebra.

See also: JuliaSparse/SparseArrays.jl#431, #52386, #52263

The performance reasons for these defs are gone, but there may be ambiguity reasons.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:excision Removal of code from Base or the repository needs pkgeval Tests for all registered packages should be run with this change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants