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

Refactor libarena #67304

Closed
wants to merge 14 commits into from
Closed

Refactor libarena #67304

wants to merge 14 commits into from

Conversation

cjgillot
Copy link
Contributor

Starting from some optimisation in libarena, I got a bit carried away.

This PR splits the logic into 4 basic allocation policies, depending on whether the allocated type is a ZST or needs dropping. I think splitting everything allows for more local reasoning, which is good for such tricky code.

A few tests have been added and bugs removed.
I am still unhappy with the tests for alloc_from_iter methods.

r? @Zoxc

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

The job mingw-check of your PR failed (pretty log, 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.
2019-12-14T23:24:36.2780507Z ##[command]git remote add origin https://github.com/rust-lang/rust
2019-12-14T23:24:36.8098211Z ##[command]git config gc.auto 0
2019-12-14T23:24:36.8102277Z ##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
2019-12-14T23:24:36.8107220Z ##[command]git config --get-all http.proxy
2019-12-14T23:24:36.8111011Z ##[command]git -c http.extraheader="AUTHORIZATION: basic ***" fetch --force --tags --prune --progress --no-recurse-submodules --depth=2 origin +refs/heads/*:refs/remotes/origin/* +refs/pull/67304/merge:refs/remotes/pull/67304/merge
---
2019-12-14T23:31:16.3459502Z     Checking rustc_data_structures v0.0.0 (/checkout/src/librustc_data_structures)
2019-12-14T23:31:17.9130756Z error[E0624]: method `current_num_threads` is private
2019-12-14T23:31:17.9131383Z    --> src/librustc_data_structures/sync.rs:440:39
2019-12-14T23:31:17.9132007Z     |
2019-12-14T23:31:17.9132887Z 440 |                 SharedWorkerLocal((0..Registry::current_num_threads()).map(|i| f(i)).collect())
2019-12-14T23:31:17.9134461Z 
2019-12-14T23:31:17.9207962Z error: aborting due to previous error
2019-12-14T23:31:17.9244710Z 
2019-12-14T23:31:17.9245709Z For more information about this error, try `rustc --explain E0624`.
2019-12-14T23:31:17.9245709Z For more information about this error, try `rustc --explain E0624`.
2019-12-14T23:31:17.9285132Z error: could not compile `rustc_data_structures`.
2019-12-14T23:31:17.9285459Z warning: build failed, waiting for other jobs to finish...
2019-12-14T23:31:28.1744213Z error: build failed
2019-12-14T23:31:28.1774189Z command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "check" "-Zconfig-profile" "--target" "x86_64-unknown-linux-gnu" "-Zbinary-dep-depinfo" "-j" "2" "--release" "--color" "always" "--features" " llvm" "--manifest-path" "/checkout/src/rustc/Cargo.toml" "--message-format" "json-render-diagnostics"
2019-12-14T23:31:28.1785767Z failed to run: /checkout/obj/build/bootstrap/debug/bootstrap check
2019-12-14T23:31:28.1786009Z Build completed unsuccessfully in 0:03:42
2019-12-14T23:31:28.1839843Z == clock drift check ==
2019-12-14T23:31:28.1853320Z   local time: Sat Dec 14 23:31:28 UTC 2019
2019-12-14T23:31:28.1853320Z   local time: Sat Dec 14 23:31:28 UTC 2019
2019-12-14T23:31:28.4619487Z   network time: Sat, 14 Dec 2019 23:31:28 GMT
2019-12-14T23:31:28.4628870Z == end clock drift check ==
2019-12-14T23:31:29.6912837Z 
2019-12-14T23:31:29.7020620Z ##[error]Bash exited with code '1'.
2019-12-14T23:31:29.7053223Z ##[section]Starting: Checkout
2019-12-14T23:31:29.7055044Z ==============================================================================
2019-12-14T23:31:29.7055101Z Task         : Get sources
2019-12-14T23:31:29.7055168Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.

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-7 of your PR failed (pretty log, 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.
2019-12-15T10:32:48.8297819Z ##[command]git remote add origin https://github.com/rust-lang/rust
2019-12-15T10:32:48.8310706Z ##[command]git config gc.auto 0
2019-12-15T10:32:48.8314071Z ##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
2019-12-15T10:32:48.8317305Z ##[command]git config --get-all http.proxy
2019-12-15T10:32:48.8322301Z ##[command]git -c http.extraheader="AUTHORIZATION: basic ***" fetch --force --tags --prune --progress --no-recurse-submodules --depth=2 origin +refs/heads/*:refs/remotes/origin/* +refs/pull/67304/merge:refs/remotes/pull/67304/merge
---
2019-12-15T11:24:16.4455737Z .................................................................................................... 1600/9380
2019-12-15T11:24:20.1820119Z .................................................................................................... 1700/9380
2019-12-15T11:24:30.5614587Z ...................................................................i................................ 1800/9380
2019-12-15T11:24:36.6517092Z .................................................................................................... 1900/9380
2019-12-15T11:24:49.7928993Z ....................................................iiiii........................................... 2000/9380
2019-12-15T11:24:58.8954678Z .................................................................................................... 2200/9380
2019-12-15T11:25:01.0320325Z .................................................................................................... 2300/9380
2019-12-15T11:25:03.9679774Z .................................................................................................... 2400/9380
2019-12-15T11:25:23.6852725Z .................................................................................................... 2500/9380
---
2019-12-15T11:27:39.4192676Z .............................................................i...............i...................... 4800/9380
2019-12-15T11:27:45.7358994Z .................................................................................................... 4900/9380
2019-12-15T11:27:53.1585097Z .................................................................................................... 5000/9380
2019-12-15T11:27:57.6989654Z .....i.............................................................................................. 5100/9380
2019-12-15T11:28:06.8611285Z .......................................................................ii.ii...........i............ 5200/9380
2019-12-15T11:28:14.5017304Z .......i............................................................................................ 5400/9380
2019-12-15T11:28:23.0498223Z .................................................................................................... 5500/9380
2019-12-15T11:28:28.6028741Z .....................................................i.............................................. 5600/9380
2019-12-15T11:28:34.5285447Z .................................................................................................... 5700/9380
2019-12-15T11:28:34.5285447Z .................................................................................................... 5700/9380
2019-12-15T11:28:43.0505675Z .................................................................................................... 5800/9380
2019-12-15T11:28:49.2016184Z .........................................ii...i..ii...........i..................................... 5900/9380
2019-12-15T11:29:08.1754822Z .................................................................................................... 6100/9380
2019-12-15T11:29:15.0334185Z .................................................................................................... 6200/9380
2019-12-15T11:29:15.0334185Z .................................................................................................... 6200/9380
2019-12-15T11:29:23.2836969Z ..................................................................i..ii............................. 6300/9380
2019-12-15T11:29:47.5162708Z .................................................................................................... 6500/9380
2019-12-15T11:29:49.3324047Z ......................................i............................................................. 6600/9380
2019-12-15T11:29:51.2152049Z .................................................................................................... 6700/9380
2019-12-15T11:29:53.2184683Z ..............................i..................................................................... 6800/9380
---
2019-12-15T11:31:17.7949963Z .................................................................................................... 7400/9380
2019-12-15T11:31:22.2146655Z .................................................................................................... 7500/9380
2019-12-15T11:31:26.7418629Z .................................................................................................... 7600/9380
2019-12-15T11:31:35.2741995Z .................................................................................................... 7700/9380
2019-12-15T11:31:43.1960978Z ...................................................iiii............................................. 7800/9380
2019-12-15T11:31:56.0809324Z .................................................................................................... 8000/9380
2019-12-15T11:32:01.5438316Z .................................................................................................... 8100/9380
2019-12-15T11:32:15.2800052Z .................................................................................................... 8200/9380
2019-12-15T11:32:22.0777334Z .................................................................................................... 8300/9380
---
2019-12-15T11:34:24.5260812Z  finished in 5.427
2019-12-15T11:34:24.5418906Z Check compiletest suite=codegen mode=codegen (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
2019-12-15T11:34:24.7287303Z 
2019-12-15T11:34:24.7288194Z running 166 tests
2019-12-15T11:34:27.3659101Z iiii......i........ii..iiii...i.............................i..i..................i....i............ 100/166
2019-12-15T11:34:29.1093498Z i.i.i...iii..iiiiiii.......................iii............ii......
2019-12-15T11:34:29.1098546Z 
2019-12-15T11:34:29.1104834Z  finished in 4.568
2019-12-15T11:34:29.1275884Z Check compiletest suite=codegen-units mode=codegen-units (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
2019-12-15T11:34:29.2758150Z 
---
2019-12-15T11:34:31.0300874Z  finished in 1.902
2019-12-15T11:34:31.0453903Z Check compiletest suite=assembly mode=assembly (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
2019-12-15T11:34:31.1939024Z 
2019-12-15T11:34:31.1939245Z running 9 tests
2019-12-15T11:34:31.1940001Z iiiiiiiii
2019-12-15T11:34:31.1940478Z 
2019-12-15T11:34:31.1944706Z  finished in 0.149
2019-12-15T11:34:31.2101983Z Check compiletest suite=incremental mode=incremental (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
2019-12-15T11:34:31.3702705Z 
---
2019-12-15T11:34:47.4358991Z  finished in 16.225
2019-12-15T11:34:47.4544584Z Check compiletest suite=debuginfo mode=debuginfo-gdb+lldb (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
2019-12-15T11:34:47.6133662Z 
2019-12-15T11:34:47.6133865Z running 124 tests
2019-12-15T11:35:08.6338694Z .iiiii..ii.....i..i...i..i.i.i..i..i..iii....ii.ii....ii..........iiii..........i.....i..ii.......ii 100/124
2019-12-15T11:35:12.0038237Z .i.iii.....iiiiii.....ii
2019-12-15T11:35:12.0039581Z 
2019-12-15T11:35:12.0039737Z  finished in 24.549
2019-12-15T11:35:12.0046365Z Uplifting stage1 rustc (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
2019-12-15T11:35:12.0048846Z Copying stage2 rustc from stage1 (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu / x86_64-unknown-linux-gnu)
---
2019-12-15T11:35:50.3756107Z ---- [ui] ui-fulldeps/dropck-tarena-unsound-drop.rs stdout ----
2019-12-15T11:35:50.3756344Z 
2019-12-15T11:35:50.3756514Z error: ui test compiled successfully!
2019-12-15T11:35:50.3756676Z status: exit code: 0
2019-12-15T11:35:50.3757571Z command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/ui-fulldeps/dropck-tarena-unsound-drop.rs" "-Zthreads=1" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "-Zui-testing" "-C" "prefer-dynamic" "--out-dir" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui-fulldeps/dropck-tarena-unsound-drop" "-Crpath" "-O" "-Cdebuginfo=0" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui-fulldeps/dropck-tarena-unsound-drop/auxiliary" "-A" "unused"
2019-12-15T11:35:50.3760177Z ------------------------------------------
2019-12-15T11:35:50.3761726Z 
2019-12-15T11:35:50.3762895Z ------------------------------------------
2019-12-15T11:35:50.3763289Z stderr:
---
2019-12-15T11:35:50.3765434Z ---- [ui] ui-fulldeps/dropck-tarena-cycle-checked.rs stdout ----
2019-12-15T11:35:50.3765646Z 
2019-12-15T11:35:50.3765829Z error: ui test compiled successfully!
2019-12-15T11:35:50.3766022Z status: exit code: 0
2019-12-15T11:35:50.3767119Z command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/ui-fulldeps/dropck-tarena-cycle-checked.rs" "-Zthreads=1" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "-Zui-testing" "-C" "prefer-dynamic" "--out-dir" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui-fulldeps/dropck-tarena-cycle-checked" "-Crpath" "-O" "-Cdebuginfo=0" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui-fulldeps/dropck-tarena-cycle-checked/auxiliary" "-A" "unused"
2019-12-15T11:35:50.3768287Z ------------------------------------------
2019-12-15T11:35:50.3768516Z 
2019-12-15T11:35:50.3769133Z ------------------------------------------
2019-12-15T11:35:50.3769366Z stderr:
---
2019-12-15T11:35:50.3772803Z test result: FAILED. 63 passed; 2 failed; 0 ignored; 0 measured; 0 filtered out
2019-12-15T11:35:50.3773026Z 
2019-12-15T11:35:50.3773249Z 
2019-12-15T11:35:50.3773403Z 
2019-12-15T11:35:50.3775107Z command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/compiletest" "--compile-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib" "--run-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib" "--rustc-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "--src-base" "/checkout/src/test/ui-fulldeps" "--build-base" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui-fulldeps" "--stage-id" "stage2-x86_64-unknown-linux-gnu" "--mode" "ui" "--target" "x86_64-unknown-linux-gnu" "--host" "x86_64-unknown-linux-gnu" "--llvm-filecheck" "/usr/lib/llvm-7/bin/FileCheck" "--host-rustcflags" "-Crpath -O -Cdebuginfo=0 -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--target-rustcflags" "-Crpath -O -Cdebuginfo=0 -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--docck-python" "/usr/bin/python2.7" "--lldb-python" "/usr/bin/python2.7" "--gdb" "/usr/bin/gdb" "--quiet" "--llvm-version" "7.0.0\n" "--system-llvm" "--cc" "" "--cxx" "" "--cflags" "" "--llvm-components" "" "--llvm-cxxflags" "" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--color" "always"
2019-12-15T11:35:50.3776000Z 
2019-12-15T11:35:50.3776176Z 
2019-12-15T11:35:50.3776352Z failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
2019-12-15T11:35:50.3776546Z Build completed unsuccessfully in 0:57:29
2019-12-15T11:35:50.3776546Z Build completed unsuccessfully in 0:57:29
2019-12-15T11:35:50.3832587Z == clock drift check ==
2019-12-15T11:35:50.3855675Z   local time: Sun Dec 15 11:35:50 UTC 2019
2019-12-15T11:35:50.6678614Z   network time: Sun, 15 Dec 2019 11:35:50 GMT
2019-12-15T11:35:50.6678723Z == end clock drift check ==
2019-12-15T11:35:52.4837902Z 
2019-12-15T11:35:52.4929947Z ##[error]Bash exited with code '1'.
2019-12-15T11:35:52.4985120Z ##[section]Starting: Checkout
2019-12-15T11:35:52.4986850Z ==============================================================================
2019-12-15T11:35:52.4986915Z Task         : Get sources
2019-12-15T11:35:52.4986957Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.

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)

@Zoxc
Copy link
Contributor

Zoxc commented Dec 22, 2019

This seems to a lot more complex with new traits and macros though. The code is already split into the 2 code paths we care about (types with destructors and types without destructors). arena.rs will dispatch to the relevant arena type and it looks like that functionality is duplicated here. The compiler doesn't allocate ZST for obvious reasons so we can just panic on those.

SyncDroplessArena does add another path for types with destructors. I do hope we'll be able to get rid of it. We might be able to refactor the need for it away right now actually. We'd have to implement Lift using interners instead of in_arena.

I think you should split out the tests to a separate PR. Fortify write_from_iter could also be a separate PR, but I'd have it be a debug assertion before returning since that code is hot.

@bors
Copy link
Contributor

bors commented Dec 23, 2019

☔ The latest upstream changes (presumably #67540) made this pull request unmergeable. Please resolve the merge conflicts.

@JohnTitor JohnTitor added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 12, 2020
@JohnCSimon
Copy link
Member

Ping from triage:
@cjgillot - can you please address the merge conflicts in this PR, please? Thank you.

@Zoxc
Copy link
Contributor

Zoxc commented Jan 18, 2020

SyncDroplessArena was removed in #67791 so that would reduce the need for this PR.

@cjgillot cjgillot closed this Jan 18, 2020
@cjgillot cjgillot deleted the arena branch January 22, 2020 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants