Improve a few vectors - calculate capacity or build from iterators #52703

ljedrz · 2018-07-25T14:43:46Z

Collecting from iterators improves readability and tailoring vector capacities should be beneficial in terms of performance.

rust-highfive · 2018-07-25T14:43:58Z

r? @nikomatsakis

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

scottmcm · 2018-07-26T06:35:20Z

src/librustc/traits/object_safety.rs

-            }
-        }
+        let violations = traits::supertrait_def_ids(self, trait_def_id)
+            .filter(|&def_id| self.predicates_reference_self(def_id, true))


I suspect that this one doesn't actually help, since filter gives a lower bound of zero:

rust/src/libcore/iter/mod.rs

Lines 1514 to 1517 in fefe816

fn size_hint(&self) -> (usize, Option<usize>) {

let (_, upper) = self.iter.size_hint();

(0, upper) // can't know a lower bound, due to the predicate

}

And thus the collect will be essentially useless at preallocating the Vec:

rust/src/liballoc/vec.rs

Lines 1852 to 1853 in fefe816

let (lower, _) = iterator.size_hint();

let mut vector = Vec::with_capacity(lower.saturating_add(1));

Fair point; I'd still consider it, though, since it improves readability and is almost a half shorter.

scottmcm · 2018-07-26T06:37:34Z

src/librustc_codegen_llvm/back/link.rs

@@ -885,9 +886,9 @@ fn exec_linker(sess: &Session, cmd: &mut Command, out_filename: &Path, tmpdir: &
    }
    let file = tmpdir.join("linker-arguments");
    let bytes = if sess.target.target.options.is_like_msvc {
-        let mut out = vec![];
+        let mut out = Vec::with_capacity(1 + args.len());


It looks like this will hold the bytes of UTF-16; should it then be 2*args.len?

Actually it should be (1 + args.len()) * 2, as each step of the for loop pushes 2 u8s. Thanks, I missed that.

rust-highfive · 2018-07-26T08:48:21Z

The job x86_64-gnu-llvm-5.0 of 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.

/usr/local/lib/python2.7/dist-packages/pip/_vendor/requests/packages/urllib3/util/ssl_.py:122: InsecurePlatformWarning: A true SSLContext object is not available. This prevents urllib3 from configuring SSL appropriately and may cause certain SSL connections to fail. You can upgrade to a newer version of Python to solve this. For more information, see https://urllib3.readthedocs.io/en/latest/security.html#insecureplatformwarning.
  InsecurePlatformWarning
/usr/local/lib/python2.7/dist-packages/pip/_vendor/requests/packages/urllib3/util/ssl_.py:122: InsecurePlatformWarning: A true SSLContext object is not available. This prevents urllib3 from configuring SSL appropriately and may cause certain SSL connections to fail. You can upgrade to a newer version of Python to solve this. For more information, see https://urllib3.readthedocs.io/en/latest/security.html#insecureplatformwarning.
  InsecurePlatformWarning
  Downloading https://files.pythonhosted.org/packages/1c/95/6bf862d284c9117eb50b3832cf39780248744e2bda3bbbcd9c5f12c489bf/awscli-1.15.65-py2.py3-none-any.whl (1.3MB)
    0% |▎                               | 10kB 12.4MB/s eta 0:00:01
    1% |▌                               | 20kB 1.9MB/s eta 0:00:01
    2% |▊                               | 30kB 2.2MB/s eta 0:00:01
    3% |█                               | 40kB 2.0MB/s eta 0:00:01
---
travis_time:end:12c8f55c:start=1532594898314189371,finish=1532594898322614950,duration=8425579
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:0fd34700
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:1500dc96
travis_time:start:1500dc96
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:148c1980
$ dmesg | grep -i kill

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)

ljedrz · 2018-07-26T09:12:15Z

Spurious error.

ljedrz · 2018-07-26T18:02:31Z

@kennytm can you trigger a retest pls?

kennytm · 2018-07-26T18:13:35Z

@ljedrz you could always re-trigger by reopening the PR.

ljedrz · 2018-07-26T18:19:15Z

@kennytm didn't know that, thanks!

rust-highfive · 2018-07-26T19:30:51Z

The job x86_64-gnu-llvm-5.0 of 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.

[01:16:20] 
[01:16:20] ..............F.
[01:16:20] failures:
[01:16:20] 
[01:16:20] ---- util::common::test_to_readable_str stdout ----
[01:16:20] thread 'util::common::test_to_readable_str' panicked at 'assertion failed: `(left == right)`
[01:16:20]   left: `"1"`,
[01:16:20]  right: `"0_001"`', librustc/util/common.rs:390:5
[01:16:20] 
[01:16:20] 
[01:16:20] failures:
[01:16:20]     util::common::test_to_readable_str
[01:16:20]     util::common::test_to_readable_str
[01:16:20] 
[01:16:20] test result: FAILED. 15 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out
[01:16:20] 
[01:16:20] error: test failed, to rerun pass '--lib'
[01:16:20] 
[01:16:20] 
[01:16:20] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "test" "--target" "x86_64-unknown-linux-gnu" "-j" "4" "--release" "--locked" "--color" "always" "--features" " jemalloc" "--manifest-path" "/checkout/src/rustc/Cargo.toml" "-p" "rustc" "--" "--quiet"
[01:16:20] 
[01:16:20] 
[01:16:20] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
[01:16:20] Build completed unsuccessfully in 0:27:42
[01:16:20] Build completed unsuccessfully in 0:27:42
[01:16:20] Makefile:58: recipe for target 'check' failed
[01:16:20] make: *** [check] Error 1

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:1adb39dc
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
---
travis_time:end:0545067f:start=1532633446224908825,finish=1532633446232158145,duration=7249320
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:30728344
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:08a6b2bc
travis_time:start:08a6b2bc
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:0107aff4
$ dmesg | grep -i kill

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)

nikomatsakis · 2018-07-28T04:52:15Z

@bors r+

bors · 2018-07-28T04:52:16Z

📌 Commit acd38f6 has been approved by nikomatsakis

…akis Improve a few vectors - calculate capacity or build from iterators Collecting from iterators improves readability and tailoring vector capacities should be beneficial in terms of performance.

@ghost

Rollup of 11 pull requests Successful merges: - #52702 (Suggest fix when encountering different mutability from impl to trait) - #52703 (Improve a few vectors - calculate capacity or build from iterators) - #52740 (Suggest underscore when using dashes in crate namet push fork) - #52759 (Impl Send & Sync for JoinHandle) - #52760 (rustc_metadata: test loading atoi instead of cos) - #52763 (Omit the vendor component in Fuchsia triple) - #52765 (Remove unused "-Zenable_nonzeroing_move_hints" flag) - #52769 (Incorporate a stray test) - #52777 (Fix doc comment for 'ptr::copy_to' method) - #52779 (revert accidental atty downgrade) - #52781 (Use a slice where a vector is not necessary) Failed merges: r? @ghost

rust-highfive assigned nikomatsakis Jul 25, 2018

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 25, 2018

scottmcm reviewed Jul 26, 2018

View reviewed changes

ljedrz force-pushed the vec_improvements branch from 3d94389 to 766ff27 Compare July 26, 2018 07:42

kennytm closed this Jul 26, 2018

kennytm reopened this Jul 26, 2018

Improve a few vectors - calculate capacity or build from iterators

acd38f6

ljedrz force-pushed the vec_improvements branch from 766ff27 to acd38f6 Compare July 26, 2018 19:49

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 Jul 28, 2018

kennytm mentioned this pull request Jul 28, 2018

Rollup of 11 pull requests #52802

Merged

bors merged commit acd38f6 into rust-lang:master Jul 28, 2018

ljedrz deleted the vec_improvements branch July 28, 2018 11:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve a few vectors - calculate capacity or build from iterators #52703

Improve a few vectors - calculate capacity or build from iterators #52703

ljedrz commented Jul 25, 2018 •

edited

Loading

rust-highfive commented Jul 25, 2018

scottmcm Jul 26, 2018

ljedrz Jul 26, 2018 •

edited

Loading

scottmcm Jul 26, 2018

ljedrz Jul 26, 2018

rust-highfive commented Jul 26, 2018

ljedrz commented Jul 26, 2018

ljedrz commented Jul 26, 2018

kennytm commented Jul 26, 2018

ljedrz commented Jul 26, 2018

rust-highfive commented Jul 26, 2018

nikomatsakis commented Jul 28, 2018

bors commented Jul 28, 2018

	fn size_hint(&self) -> (usize, Option<usize>) {
	let (_, upper) = self.iter.size_hint();
	(0, upper) // can't know a lower bound, due to the predicate
	}

	let (lower, _) = iterator.size_hint();
	let mut vector = Vec::with_capacity(lower.saturating_add(1));

Improve a few vectors - calculate capacity or build from iterators #52703

Improve a few vectors - calculate capacity or build from iterators #52703

Conversation

ljedrz commented Jul 25, 2018 • edited Loading

rust-highfive commented Jul 25, 2018

scottmcm Jul 26, 2018

Choose a reason for hiding this comment

ljedrz Jul 26, 2018 • edited Loading

Choose a reason for hiding this comment

scottmcm Jul 26, 2018

Choose a reason for hiding this comment

ljedrz Jul 26, 2018

Choose a reason for hiding this comment

rust-highfive commented Jul 26, 2018

ljedrz commented Jul 26, 2018

ljedrz commented Jul 26, 2018

kennytm commented Jul 26, 2018

ljedrz commented Jul 26, 2018

rust-highfive commented Jul 26, 2018

nikomatsakis commented Jul 28, 2018

bors commented Jul 28, 2018

ljedrz commented Jul 25, 2018 •

edited

Loading

ljedrz Jul 26, 2018 •

edited

Loading