Skip to content

Commit

Permalink
speed up sortperm. ref #6112
Browse files Browse the repository at this point in the history
on my system, now even faster than before
  • Loading branch information
JeffBezanson committed Jul 3, 2014
1 parent a40bc88 commit e8b6c45
Showing 1 changed file with 3 additions and 2 deletions.
5 changes: 3 additions & 2 deletions base/ordering.jl
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,9 @@ lt(o::Lt, a, b) = o.lt(a,b)
lt(o::LexicographicOrdering, a, b) = lexcmp(a,b) < 0

function lt(p::Perm, a::Int, b::Int)
lt(p.order, p.data[a], p.data[b]) ? true :
lt(p.order, p.data[b], p.data[a]) ? false : a < b
da = p.data[a]
db = p.data[b]
lt(p.order, da, db) | (!lt(p.order, db, da) & (a < b))
end
function lt(p::Perm{LexicographicOrdering}, a::Int, b::Int)
c = lexcmp(p.data[a], p.data[b])
Expand Down

4 comments on commit e8b6c45

@StefanKarpinski
Copy link
Member

Choose a reason for hiding this comment

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

Oh good. Kind of a shaky that this needs to be manually hoisted, but whatever. We should time against Matlab again.

@StefanKarpinski
Copy link
Member

Choose a reason for hiding this comment

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

Shaky => shame

@JeffBezanson
Copy link
Member Author

Choose a reason for hiding this comment

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

Actually it works with or without the hoisting. Removing the branches is the key. It is way faster now.

@ViralBShah
Copy link
Member

Choose a reason for hiding this comment

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

Great. This is an important function to have good performance for.

Please sign in to comment.