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

Reduce redundant module linking #3228

Merged
merged 6 commits into from Oct 17, 2018
Merged

Conversation

sklam
Copy link
Member

@sklam sklam commented Aug 15, 2018

Reduce compilation time by removing redundant module linking. Avoid putting dependent module into global list of modules to link. Observed 5x reduction of link_in calls for the "important" tests.

Adds a stack of CodeLibrary into the target-context, which can now query the active code-library via . active_code_library.

Note about the "global list of modules to link":

The CodeGen maintains a list of modules required for all new compilation unit for that target. These modules are linked-into all future functions. Before this patch, any function created from compile_internal is being added to the global list instead of the local one. This means any new function will have to link against any compile_internal function regardless of whether they needed it or now.

@codecov-io
Copy link

codecov-io commented Aug 15, 2018

Codecov Report

Merging #3228 into master will increase coverage by 0.02%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #3228      +/-   ##
==========================================
+ Coverage   80.57%   80.59%   +0.02%     
==========================================
  Files         390      390              
  Lines       78248    78349     +101     
  Branches     8834     8854      +20     
==========================================
+ Hits        63045    63145     +100     
+ Misses      13821    13808      -13     
- Partials     1382     1396      +14

@sklam sklam changed the title [WIP] Reduce redundant module linking Reduce redundant module linking Aug 15, 2018
@stuartarchibald stuartarchibald added this to the Numba 0.40 RC milestone Aug 22, 2018
Copy link
Contributor

@stuartarchibald stuartarchibald left a comment

Choose a reason for hiding this comment

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

Thanks for this patch. The implementation makes sense and should improve performance especially for more involved compilations. As discussed over IRC, this should be threadsafe by virtue of the global compiler lock and atomic append/pop on list. Couple of minor comments but otherwise good to go. Thanks.

return cres

def compile_subroutine(self, builder, impl, sig, locals={}):
def compile_subroutine(self, builder, impl, sig, locals={}, flags=None,
caching=True):
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps add what caching does to the docstring for this function?

Copy link
Member Author

Choose a reason for hiding this comment

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

added docstring

def active_code_library(self):
"""Get the active code library
"""
return self._codelib_stack[-1]
Copy link
Contributor

Choose a reason for hiding this comment

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

Could someone end up calling this before the _codelib_stack contains at least one entry? Worth catching it?

Copy link
Member Author

Choose a reason for hiding this comment

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

If that happens, it is an internal error.

@seibert seibert added this to In Progress in Minor Features Aug 27, 2018
@seibert
Copy link
Contributor

seibert commented Sep 14, 2018

@sklam: This PR needs to resolve conflicts with master now.

# Conflicts:
#	numba/compiler.py
#	numba/targets/base.py
@stuartarchibald
Copy link
Contributor

Thanks for the fix ups, this can be merged as convenient.

@stuartarchibald stuartarchibald merged commit b6fcc8c into numba:master Oct 17, 2018
Minor Features automation moved this from In Progress to Done Oct 17, 2018
@sklam sklam deleted the enh/reducelinkin branch November 13, 2018 21:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants