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

[WIP] Use buffered aHash as StableHasher #59598

Closed
wants to merge 2 commits into from
Closed

Conversation

Zoxc
Copy link
Contributor

@Zoxc Zoxc commented Apr 1, 2019

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 1, 2019
@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-llvm-6.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.
travis_time:end:1c7e158c:start=1554089830444271833,finish=1554089831398847902,duration=954576069
$ git checkout -qf FETCH_HEAD
travis_fold:end:git.checkout

Encrypted environment variables have been removed for security reasons.
See https://docs.travis-ci.com/user/pull-requests/#pull-requests-and-security-restrictions
$ export SCCACHE_BUCKET=rust-lang-ci-sccache2
$ export SCCACHE_REGION=us-west-1
$ export GCP_CACHE_BUCKET=rust-lang-ci-cache
Setting environment variables from .travis.yml
---

[00:04:05] travis_fold:start:tidy
travis_time:start:tidy
tidy check
[00:04:06] tidy error: /checkout/src/librustc_data_structures/convert.rs: missing trailing newline
[00:04:07] Dependencies not on the whitelist:
[00:04:07] some tidy checks failed
[00:04:07] 
[00:04:07] 
[00:04:07] 
[00:04:07] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/tidy" "/checkout/src" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "--no-vendor" "--quiet"
[00:04:07] 
[00:04:07] 
[00:04:07] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
[00:04:07] Build completed unsuccessfully in 0:00:48
[00:04:07] Build completed unsuccessfully in 0:00:48
[00:04:07] Makefile:67: recipe for target 'tidy' failed
[00:04:07] make: *** [tidy] Error 1
The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:0e3cb967
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
Mon Apr  1 03:41:30 UTC 2019
---
travis_time:end:1b353d96:start=1554090090977372568,finish=1554090090982724972,duration=5352404
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:0d2dd22a
$ 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 --batch -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:1ae361a0
travis_time:start:1ae361a0
$ 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:02458818
$ 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)

@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-llvm-6.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.
travis_time:end:0b6c0600:start=1554092008640549225,finish=1554092009705376990,duration=1064827765
$ git checkout -qf FETCH_HEAD
travis_fold:end:git.checkout

Encrypted environment variables have been removed for security reasons.
See https://docs.travis-ci.com/user/pull-requests/#pull-requests-and-security-restrictions
$ export SCCACHE_BUCKET=rust-lang-ci-sccache2
$ export SCCACHE_REGION=us-west-1
$ export GCP_CACHE_BUCKET=rust-lang-ci-cache
Setting environment variables from .travis.yml
---
[00:04:16] * arrayref 
[00:04:16] some tidy checks failed
[00:04:16] 
[00:04:16] 
[00:04:16] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/tidy" "/checkout/src" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "--no-vendor" "--quiet"
[00:04:16] 
[00:04:16] 
[00:04:16] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
[00:04:16] Build completed unsuccessfully in 0:00:46
[00:04:16] Build completed unsuccessfully in 0:00:46
[00:04:16] Makefile:67: recipe for target 'tidy' failed
[00:04:16] make: *** [tidy] Error 1
The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:16324768
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
Mon Apr  1 04:17:57 UTC 2019
---
travis_time:end:024e12c2:start=1554092278506085663,finish=1554092278511379299,duration=5293636
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:042c45c6
$ 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 --batch -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:04249b7e
travis_time:start:04249b7e
$ 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:03caff10
$ 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)

@michaelwoerister
Copy link
Member

Is this hash function documented anywhere? Does it provide roughly the same collision probability as a random function? Are its results platform independent?

Some previous discussion on which hash function to use: #41215

@Zoxc
Copy link
Contributor Author

Zoxc commented Apr 3, 2019

I just aimed for a simpler question? Is it faster? Local testing says no, at least with the buffering scheme and runtime AES instruction detection used here.

@michaelwoerister
Copy link
Member

I just aimed for a simpler question? Is it faster?

Yes, I'm always interested in numbers there :)

Although ideally, it would be great to get rid of hashing altogether in the long run: #58155

@tkaitchuck
Copy link
Contributor

@michaelwoerister You can see the documentation here: https://github.com/tkaitchuck/ahash
It is certainly intended to be of high quality. It is certainly a higher quality hash than FxHash, but it cannot out perform it on primitive types, where FxHash will be reduced to a single multiply instruction. A more relevant comparison is SipHash-1-3 where I believe aHash to be both faster and of higher quality.

@michaelwoerister
Copy link
Member

@tkaitchuck That looks pretty neat! I don't think it's a good fit for the StableHasher though because we need platform independent results.

@Zoxc
Copy link
Contributor Author

Zoxc commented Apr 5, 2019

@michaelwoerister I don't see a reason for why we need platform independent results (at least for incremental compilation hashes), but a software fallback producing the same result is always possible.

@michaelwoerister
Copy link
Member

The StableHasher is also used for symbol names and type-id. IIRC, we've had problems in the past where platform-dependent hashing would lead to linker errors when cross-compiling.

@tkaitchuck
Copy link
Contributor

Ok. Sounds like this isn't the right fit.

@Zoxc Zoxc closed this Apr 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants