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

replace usage of slow slist #5109

Merged
merged 1 commit into from Feb 19, 2016
Merged

replace usage of slow slist #5109

merged 1 commit into from Feb 19, 2016

Conversation

MartinNowak
Copy link
Member

  • directly reset symbols after writing them to an object file
    (it's very cheap when they are still in cache)
  • external references are already reset right away
    in outfixlist
  • no need to reset symbols that cannot be referenced
    (e.g. locals, to SCenum/SCstruct symbols)
  • reset tls helper with rtlsym_reset

@MartinNowak
Copy link
Member Author

About 10% faster when compiling multilib archives.

@WalterBright
Copy link
Member

From the autotest failures, it appears that some symbols are not getting reset.

@MartinNowak MartinNowak force-pushed the slist_reset branch 4 times, most recently from dcdfc58 to b6822b5 Compare September 24, 2015 23:23
@MartinNowak
Copy link
Member Author

From the autotest failures, it appears that some symbols are not getting reset.

Unfortunately it's a bigger issue. Some obj implementations (macho, mscoff) retain a pointer to the symbols and defer writing the symbol table until all sections are written.
I worked on directly writing the symbol table (and patching it later) but it's too much work, see MartinNowak@fc413b7.
Instead it seems like a better idea to reset the symbols in the obj implementation where we know exactly which symbols were used.

@MartinNowak
Copy link
Member Author

Updated, I had overlooked an spurious symbol_reset from an earlier approach to fix this issue.

@MartinNowak
Copy link
Member Author

This seems to be a bit slower than directly resetting the symbols (prolly b/c they are out of cache when the obj file is finished), but it's dead simple and compiles static libraries 15-20% faster than w/ the current slist_reset of all symbols.
I originally tried to rewrite those obj modules that store pointers to symbols to directly convert the symbols into the target format (MartinNowak@fc413b7), but that's a major change of mscoff and macho.

@MartinNowak MartinNowak force-pushed the slist_reset branch 2 times, most recently from 38afb06 to 0cc754a Compare February 2, 2016 01:42
@MartinNowak
Copy link
Member Author

From the autotest failures, it appears that some symbols are not getting reset.

Found the issue with OMF, forgot to add pubdef symbols to the reset list.

@MartinNowak MartinNowak added this to the 2.071.0 milestone Feb 2, 2016
- directly reset symbols after putting them out to the obj file
- it's done in the obj implementations b/c some (macho, mscoff)
  defer writing the symbol table until all sections are written
@MartinNowak
Copy link
Member Author

Fixed the merge conflict.

@andralex
Copy link
Member

worksforme

@andralex
Copy link
Member

Auto-merge toggled on

andralex added a commit that referenced this pull request Feb 19, 2016
@andralex andralex merged commit 6232901 into dlang:master Feb 19, 2016
@MartinNowak MartinNowak deleted the slist_reset branch February 20, 2016 17:07
@qchikara
Copy link
Contributor

qchikara commented Mar 3, 2016

This introduces a regression:
https://issues.dlang.org/show_bug.cgi?id=15729

@CyberShadow
Copy link
Member

This pull request fixed a regression:
https://issues.dlang.org/show_bug.cgi?id=12624

Good news for a change!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants