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 IR for non-inlineable functions after codegen to save memory #16907

Merged
merged 1 commit into from
Jun 14, 2016

Conversation

JeffBezanson
Copy link
Member

@JeffBezanson JeffBezanson commented Jun 13, 2016

This saves an appreciable amount of memory.

Before:

    From worker 3:       * linalg/qr             in 109.82 seconds, maxrss  381.65 MB
    From worker 3:       * linalg/dense          in  41.32 seconds, maxrss  439.55 MB
    From worker 3:       * linalg/matmul         in 136.85 seconds, maxrss  516.54 MB
    From worker 3:       * linalg/schur          in   5.81 seconds, maxrss  521.65 MB
    From worker 3:       * linalg/special        in   3.30 seconds, maxrss  524.22 MB
    From worker 3:       * linalg/eigen          in  28.30 seconds, maxrss  537.02 MB
    From worker 3:       * linalg/bunchkaufman   in  22.59 seconds, maxrss  542.05 MB
    From worker 3:       * linalg/svd            in  12.16 seconds, maxrss  551.03 MB
    From worker 3:       * linalg/lapack         in  26.47 seconds, maxrss  556.81 MB
    From worker 2:       * linalg/triangular     in 398.41 seconds, maxrss  827.11 MB

with this change:

    From worker 3:       * linalg/qr             in  99.19 seconds, maxrss  380.03 MB
    From worker 3:       * linalg/dense          in  49.18 seconds, maxrss  421.14 MB
    From worker 3:       * linalg/matmul         in 150.76 seconds, maxrss  487.86 MB
    From worker 3:       * linalg/schur          in   6.09 seconds, maxrss  487.93 MB
    From worker 3:       * linalg/special        in   2.99 seconds, maxrss  487.93 MB
    From worker 3:       * linalg/eigen          in  27.72 seconds, maxrss  491.68 MB
    From worker 3:       * linalg/bunchkaufman   in  22.36 seconds, maxrss  494.21 MB
    From worker 3:       * linalg/svd            in  11.87 seconds, maxrss  501.07 MB
    From worker 3:       * linalg/lapack         in  28.59 seconds, maxrss  519.14 MB
    From worker 2:       * linalg/triangular     in 408.06 seconds, maxrss  775.71 MB

@vtjnash
Copy link
Member

vtjnash commented Jun 13, 2016

This seems to be barely a couple percent savings? That seems rather disappointing. Is the large slowdown (esp the 50% on linalg/dense) real or just the usual travis noise?

@JeffBezanson JeffBezanson force-pushed the jb/delete_non_inlineable branch from b229dfa to 923cdd2 Compare June 13, 2016 15:44
@JeffBezanson
Copy link
Member Author

Ah, I needed to fix the inlining logic a bit, and now the numbers look better.

@JeffBezanson JeffBezanson force-pushed the jb/delete_non_inlineable branch from 923cdd2 to e24fec2 Compare June 13, 2016 16:29
@JeffBezanson
Copy link
Member Author

@vtjnash - I think this is ready for final review.

@vtjnash
Copy link
Member

vtjnash commented Jun 14, 2016

lgtm

@JeffBezanson JeffBezanson merged commit 4c1af23 into master Jun 14, 2016
@tkelman tkelman deleted the jb/delete_non_inlineable branch June 14, 2016 04:58
@tkelman
Copy link
Contributor

tkelman commented Jun 14, 2016

This will need to be reverted if you can't propose a fix for #16921.

@JeffBezanson
Copy link
Member Author

Ok, ok, give me some time.

@JeffBezanson
Copy link
Member Author

Also, if necessary, you can change JL_DELETE_NON_INLINEABLE to 0 in options.h instead of reverting.

@vtjnash vtjnash mentioned this pull request Jun 16, 2016
if (JL_DELETE_NON_INLINEABLE &&
li->def && li->inferred && !li->inlineable && !jl_options.outputji) {
li->code = jl_nothing;
li->slottypes = jl_nothing;
Copy link
Member

Choose a reason for hiding this comment

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

Can we keep slottypes? The debugger uses them.

Copy link
Member Author

Choose a reason for hiding this comment

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

OK. You can just go ahead and take this line out.

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.

4 participants