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

proc_macro: make TokenStream::from_streams pre-allocate its vector. #58001

Conversation

pnkfelix
Copy link
Member

@pnkfelix pnkfelix commented Jan 30, 2019

This requires a pre-pass over the input streams. But that is cheap compared to the quadratic blowup associated with reallocating the accumulating vector on-the-fly.

Fix #57735

This requires a pre-pass over the input streams. But that is cheap
compared to the quadratic blowup associated with reallocating the
accumulating vector on-the-fly.
@rust-highfive
Copy link
Collaborator

r? @eddyb

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 30, 2019
@pnkfelix
Copy link
Member Author

pnkfelix commented Jan 30, 2019

Note that this does not include a regression test because it is non-trivial to reproduce the conditions of the test.

In particular, the quote! macro provided by rustc's internal proc-macro implementation does not support the repetition operator (#54722 (comment)) that is used in the original bug's test scenario (#57735 (comment))


I did try to whip up a test that didn't use quote! (just strings and the builtin parsing facilities) but those did not replicate the quadratic blowup as far as I could tell.

@pnkfelix pnkfelix added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jan 30, 2019
@pnkfelix
Copy link
Member Author

cc @nnethercote

@nnethercote
Copy link
Contributor

Thank you for digging into this, @pnkfelix! Reverting #57004 would have been sad, because it was a significant simplification, and #57486 is a follow-on, and I'm working on additional dependent changes. The change looks fine to me.

@eddyb
Copy link
Member

eddyb commented Jan 30, 2019

@bors r+

@bors
Copy link
Contributor

bors commented Jan 30, 2019

📌 Commit 1a18336 has been approved by eddyb

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 30, 2019
@bors
Copy link
Contributor

bors commented Jan 31, 2019

⌛ Testing commit 1a18336 with merge ff0c8c90fd2f2af47457e1fa0a44a9c858ff75ea...

@bors
Copy link
Contributor

bors commented Jan 31, 2019

💔 Test failed - status-appveyor

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 31, 2019
@rust-highfive
Copy link
Collaborator

Your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
travis_fold:end:before_install.3
travis_fold:start:install
travis_time:start:08dbcc91
$ case "$TRAVIS_OS_NAME" in linux) travis_retry curl -fo $HOME/stamp https://s3-us-west-1.amazonaws.com/rust-lang-ci2/rust-ci-mirror/2017-03-17-stamp-x86_64-unknown-linux-musl && chmod +x $HOME/stamp && export PATH=$PATH:$HOME ;; osx) if [[ "$RUST_CHECK_TARGET" == dist ]]; then travis_retry brew update && travis_retry brew install xz && travis_retry brew install swig; fi && travis_retry curl -fo /usr/local/bin/sccache https://s3-us-west-1.amazonaws.com/rust-lang-ci2/rust-ci-mirror/2018-04-02-sccache-x86_64-apple-darwin && chmod +x /usr/local/bin/sccache && travis_retry curl -fo /usr/local/bin/stamp https://s3-us-west-1.amazonaws.com/rust-lang-ci2/rust-ci-mirror/2017-03-17-stamp-x86_64-apple-darwin && chmod +x /usr/local/bin/stamp && travis_retry curl -f http://releases.llvm.org/7.0.0/clang+llvm-7.0.0-x86_64-apple-darwin.tar.xz | tar xJf - && export CC=`pwd`/clang+llvm-7.0.0-x86_64-apple-darwin/bin/clang && export CXX=`pwd`/clang+llvm-7.0.0-x86_64-apple-darwin/bin/clang++ && export AR=ar ;; esac
curl: (56) Recv failure: Connection reset by peer
The command "curl -f http://releases.llvm.org/7.0.0/clang+llvm-7.0.0-x86_64-apple-darwin.tar.xz" failed. Retrying, 2 of 3.
clang+llvm-7.0.0-x86_64-apple-darwin/bin/llvm-dwp: Lzma library error: Corrupted input data
tar: Error exit delayed from previous errors.
travis_time:end:08dbcc91:start=1548916473237336000,finish=1548916501064432000,duration=27827096000
The command "case "$TRAVIS_OS_NAME" in linux) travis_retry curl -fo $HOME/stamp https://s3-us-west-1.amazonaws.com/rust-lang-ci2/rust-ci-mirror/2017-03-17-stamp-x86_64-unknown-linux-musl && chmod +x $HOME/stamp && export PATH=$PATH:$HOME ;; osx) if [[ "$RUST_CHECK_TARGET" == dist ]]; then travis_retry brew update && travis_retry brew install xz && travis_retry brew install swig; fi && travis_retry curl -fo /usr/local/bin/sccache https://s3-us-west-1.amazonaws.com/rust-lang-ci2/rust-ci-mirror/2018-04-02-sccache-x86_64-apple-darwin && chmod +x /usr/local/bin/sccache && travis_retry curl -fo /usr/local/bin/stamp https://s3-us-west-1.amazonaws.com/rust-lang-ci2/rust-ci-mirror/2017-03-17-stamp-x86_64-apple-darwin && chmod +x /usr/local/bin/stamp && travis_retry curl -f http://releases.llvm.org/7.0.0/clang+llvm-7.0.0-x86_64-apple-darwin.tar.xz | tar xJf - && export CC=`pwd`/clang+llvm-7.0.0-x86_64-apple-darwin/bin/clang && export CXX=`pwd`/clang+llvm-7.0.0-x86_64-apple-darwin/bin/clang++ && export AR=ar ;; esac" failed and exited with 1 during .
Your build has been stopped.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@Centril
Copy link
Contributor

Centril commented Jan 31, 2019

@bors retry

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 31, 2019
@bors
Copy link
Contributor

bors commented Jan 31, 2019

⌛ Testing commit 1a18336 with merge 6c02e295e76a8a271720d3d0ebf003de05882ba3...

@bors
Copy link
Contributor

bors commented Jan 31, 2019

💔 Test failed - status-appveyor

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 31, 2019
@pnkfelix
Copy link
Member Author

hmm:

[01:56:11] [RUSTC-TIMING] rustfmt_nightly test:false 59.350
[01:56:15] error[E0061]: this function takes 4 parameters but 3 parameters were supplied
[01:56:15]    --> src\tools\rls\src\build\rustc.rs:214:12
[01:56:15]     |
[01:56:15] 214 |         ls.register_early_pass(Some(sess), true, pass);
[01:56:15]     |            ^^^^^^^^^^^^^^^^^^^ expected 4 parameters
[01:56:15] 
[01:56:15] error: aborting due to previous error
[01:56:15] 
[01:56:15] For more information about this error, try `rustc --explain E0061`.
[01:56:15] [RUSTC-TIMING] rls test:false 4.517
[01:56:16] error: Could not compile `rls`.
[01:56:16] 
[01:56:16] To learn more, run the command again with --verbose.
[01:56:16] command did not execute successfully: "C:\\projects\\rust\\build\\x86_64-pc-windows-gnu\\stage0\\bin\\cargo.exe" "build" "--target" "x86_64-pc-windows-gnu" "-j" "4" "--release" "--locked" "--color" "always" "--manifest-path" "C:\\projects\\rust\\src/tools/rls\\Cargo.toml" "--features" "clippy, rustc-workspace-hack/all-static" "--message-format" "json"
[01:56:16] expected success, got: exit code: 101
[01:56:16] [TIMING] ToolBuild { compiler: Compiler { stage: 2, host: "x86_64-pc-windows-gnu" }, target: "x86_64-pc-windows-gnu", tool: "rls", path: "src/tools/rls", mode: ToolRustc, is_optional_tool: true, source_type: Submodule, extra_features: ["clippy"] } -- 310.115
[01:56:16] Unable to build RLS, skipping dist

@pnkfelix
Copy link
Member Author

pnkfelix commented Jan 31, 2019

do we need to update the Cargo.lock or something in order to link in the RLS post PR rust-lang/rls#1268 in order to get https://github.com/rust-lang/rls/pull/1268/files#diff-48b18d7151f11423d709ca98bee07cefL214 ?

@mati865
Copy link
Contributor

mati865 commented Jan 31, 2019

@pnkfelix rls submodule has to be updated but there was issue with the tests last time rust-lang/rls#1271. There is PR for that: #58024

It doesn't matter though because tools are allowed to fail.
Somehow build succeed but AppVeyor detected it as a fail:

Successful compile (293.250 sec). Resulting Setup program filename is:

#57514 #58003 failed in the same way

@kennytm
Copy link
Member

kennytm commented Feb 5, 2019

@bors retry

"Build exceeded allowed resource quotas. Fix your build to consume less resources or contact AppVeyor support to request quotas increase."

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 5, 2019
kennytm added a commit to kennytm/rust that referenced this pull request Feb 5, 2019
…h-large-tokenstream-slow, r=eddyb

proc_macro: make `TokenStream::from_streams` pre-allocate its vector.

This requires a pre-pass over the input streams. But that is cheap compared to the quadratic blowup associated with reallocating the accumulating vector on-the-fly.

Fix rust-lang#57735
bors added a commit that referenced this pull request Feb 5, 2019
Rollup of 23 pull requests

Successful merges:

 - #58001 (proc_macro: make `TokenStream::from_streams` pre-allocate its vector.)
 - #58096 (Transition linkchecker to 2018 edition)
 - #58097 (Transition remote test to Rust 2018)
 - #58106 (libfmt_macros => 2018)
 - #58107 (libgraphviz => 2018)
 - #58108 (Add NVPTX target to a build manifest)
 - #58109 (librustc_privacy => 2018)
 - #58112 (libpanic_abort => 2018)
 - #58113 (Transition build-manifest to 2018 edition)
 - #58114 (Transition tidy and unstable-book-gen to 2018 edition)
 - #58116 (Include the span of attributes of the lhs to the span of the assignment expression)
 - #58117 (Transition rustdoc-theme to 2018 edition)
 - #58128 (libunwind => 2018)
 - #58138 (Fix #58101)
 - #58139 (hir: add more HirId methods)
 - #58141 (Remove weasel word in docs for iter's take_while())
 - #58142 (Remove stray FIXME)
 - #58145 (Add #[must_use] to core::task::Poll)
 - #58162 (Add more debugging code to track down appveyor 259 exit code)
 - #58169 (Update contributor name in .mailmap)
 - #58172 (update split docs)
 - #58182 (SGX target: handle empty user buffers correctly)
 - #58186 (Add Rustlings to the doc index)

Failed merges:

r? @ghost
@bors bors merged commit 1a18336 into rust-lang:master Feb 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants