Skip to content
This repository has been archived by the owner on Apr 18, 2023. It is now read-only.

Don't force inlining--let Julia figure that out on its own #160

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

Conversation

ararslan
Copy link
Collaborator

We're currently forcibly inlining a lot of stuff. This is a draft PR to see what the ramifications would be of not doing any of that, and simply letting Julia figure out when it should inline. This is motivated by a comment from @willtebbutt: #145 (review).

Copy link
Member

@willtebbutt willtebbutt left a comment

Choose a reason for hiding this comment

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

Looks good. I would be interested to know if there's anything that significantly slows down -- it would kind of surprise me if there were.

@ararslan
Copy link
Collaborator Author

It would surprise me as well. I'll put some benchmarks together, I just need to determine what would be the applications most affected by this change (if at all).

@codecov
Copy link

codecov bot commented Apr 23, 2019

Codecov Report

Merging #160 into master will decrease coverage by <.01%.
The diff coverage is 87.87%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #160      +/-   ##
==========================================
- Coverage   97.05%   97.05%   -0.01%     
==========================================
  Files          20       20              
  Lines         748      747       -1     
==========================================
- Hits          726      725       -1     
  Misses         22       22
Impacted Files Coverage Δ
src/sensitivity.jl 100% <100%> (ø) ⬆️
src/sensitivities/scalar.jl 100% <100%> (ø) ⬆️
src/core.jl 93.61% <100%> (ø) ⬆️
src/sensitivities/functional/functional.jl 76.19% <60%> (-0.56%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fffecf5...47c6657. Read the comment docs.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants