Skip to content

Conversation

@rdeits
Copy link
Collaborator

@rdeits rdeits commented Nov 29, 2016

This yields about a 10x improvement in substitution time on my simple benchmark (included in perf/runbenchmarks.jl).

@rdeits rdeits mentioned this pull request Nov 29, 2016
@codecov-io
Copy link

codecov-io commented Nov 30, 2016

Current coverage is 70.20% (diff: 100%)

Merging #2 into master will decrease coverage by 0.67%

@@             master         #2   diff @@
==========================================
  Files            12         12          
  Lines           697        698     +1   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
- Hits            494        490     -4   
- Misses          203        208     +5   
  Partials          0          0          

Powered by Codecov. Last update 4162fa7...31d4262

@coveralls
Copy link

coveralls commented Nov 30, 2016

Coverage Status

Coverage decreased (-0.6%) to 70.418% when pulling 899b325 on performance-improvements into 98be11f on master.

@@ -1,5 +1,5 @@
function evalmap(vars, x::Vector, varorder::Vector{PolyVar})
vals = Any[var for var in vars]
function evalmap{T}(vars, x::Vector{T}, varorder::Vector{PolyVar})
Copy link
Member

@blegat blegat Nov 30, 2016

Choose a reason for hiding this comment

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

Should we do a shortcut like:

if vars == varorder
x
else
... # current body of evalmap
end

?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

good idea; done

@rdeits rdeits force-pushed the performance-improvements branch from 899b325 to 31d4262 Compare November 30, 2016 16:57
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.7%) to 70.201% when pulling 31d4262 on performance-improvements into 4162fa7 on master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.7%) to 70.201% when pulling 31d4262 on performance-improvements into 4162fa7 on master.

@rdeits
Copy link
Collaborator Author

rdeits commented Nov 30, 2016

Okay, this has been rebased and cleaned up.

@coveralls
Copy link

coveralls commented Nov 30, 2016

Coverage Status

Coverage decreased (-0.7%) to 70.201% when pulling 969be8f on performance-improvements into 4162fa7 on master.

@blegat blegat merged commit 5fe227f into master Nov 30, 2016
@blegat
Copy link
Member

blegat commented Nov 30, 2016

Thanks a lot for looking into this !

@rdeits
Copy link
Collaborator Author

rdeits commented Nov 30, 2016

No problem :). Optimizing julia code is surprisingly satisfying.

@rdeits rdeits deleted the performance-improvements branch November 30, 2016 17:34
blegat added a commit that referenced this pull request Jun 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants