-
Notifications
You must be signed in to change notification settings - Fork 13.2k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Removed dependency on the field-offset crate, alternate approach #136201
Conversation
These commits modify the If this was unintentional then you should revert the changes before this PR is merged. The list of allowed third-party dependencies may have been modified! You must ensure that any new dependencies have compatible licenses before merging. |
d8b458a
to
ef6ee83
Compare
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
…t, r=<try> Removed dependency on the field-offset crate, alternate approach This is an alternate approach to reach the same goals as rust-lang#136003. As it touches the core of the query system, this too probably should be evaluated for performance. r? `@Mark-Simulacrum`
☀️ Try build successful - checks-actions |
This comment has been minimized.
This comment has been minimized.
Finished benchmarking commit (3da1435): comparison URL. Overall result: no relevant changes - no action neededBenchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf. @bors rollup=never Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)Results (primary 1.1%, secondary 3.9%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResults (secondary -2.0%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 778.134s -> 774.743s (-0.44%) |
@bors r+ I think this approach seems pretty clean and perf looks good. |
Is there some reason |
I chose to not pursue that approach as the use of that dependency was really small in the compiler, and furthermore that project seems to be unmaintained. Though even if it weren't it seems rather excessive to me to have 2 dependencies for this. |
…t, r=Mark-Simulacrum Removed dependency on the field-offset crate, alternate approach This is an alternate approach to reach the same goals as rust-lang#136003. As it touches the core of the query system, this too probably should be evaluated for performance. r? `@Mark-Simulacrum`
This comment has been minimized.
This comment has been minimized.
💔 Test failed - checks-actions |
Thanks! @bors r=Mark-Simulacrum |
@bors rollup=maybe perf neutral |
Rollup of 5 pull requests Successful merges: - rust-lang#134777 (Enable more tests on Windows) - rust-lang#135621 (Move some std tests to integration tests) - rust-lang#135844 ( Add new tool for dumping feature status based on tidy ) - rust-lang#136167 (Implement unstable `new_range` feature) - rust-lang#136334 (Extract `core::ffi` primitives to a separate (internal) module) Failed merges: - rust-lang#136201 (Removed dependency on the field-offset crate, alternate approach) r? `@ghost` `@rustbot` modify labels: rollup
☔ The latest upstream changes (presumably #136533) made this pull request unmergeable. Please resolve the merge conflicts. |
2574281
to
62bbaa8
Compare
@rustbot ready |
@bors r=Mark-Simulacrum |
Something weird is happening with bors on this one I think. It doesn't show approved in the queue yet. |
@bors r=Mark-Simulacrum |
Should be fixed now. |
…alt, r=Mark-Simulacrum Removed dependency on the field-offset crate, alternate approach This is an alternate approach to reach the same goals as rust-lang#136003. As it touches the core of the query system, this too probably should be evaluated for performance. r? `@Mark-Simulacrum`
…kingjubilee Rollup of 12 pull requests Successful merges: - rust-lang#136053 (coverage: Defer part of counter-creation until codegen) - rust-lang#136201 (Removed dependency on the field-offset crate, alternate approach) - rust-lang#136228 (Simplify Rc::as_ptr docs + typo fix) - rust-lang#136353 (fix(libtest): Enable Instant on Emscripten targets) - rust-lang#136472 ([`compiletest`-related cleanups 2/7] Feed stage number to compiletest directly) - rust-lang#136487 (ci: stop mysql before removing it) - rust-lang#136552 (Use an `Option` for `FindNextFileHandle` in `ReadDir` instead of `INVALID_FILE_HANDLE` sentinel value) - rust-lang#136705 (Some miscellaneous edition-related library tweaks) - rust-lang#136707 (Bump `cc` to v1.2.13 for the compiler workspace) - rust-lang#136790 (Git blame ignore recent formatting commit) - rust-lang#136792 (Don't apply editorconfig to llvm) - rust-lang#136805 (ignore win_delete_self test in Miri) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#136201 - davidv1992:eliminate-field-offset-alt, r=Mark-Simulacrum Removed dependency on the field-offset crate, alternate approach This is an alternate approach to reach the same goals as rust-lang#136003. As it touches the core of the query system, this too probably should be evaluated for performance. r? ``@Mark-Simulacrum``
This is an alternate approach to reach the same goals as #136003. As it touches the core of the query system, this too probably should be evaluated for performance.
r? @Mark-Simulacrum