[PATCH core-updates] gnu: rust: Add profiling support.

  • Done
  • quality assurance status badge
Details
3 participants
  • Marius Bakke
  • Maxim Cournoyer
  • Milkey Mouse
Owner
unassigned
Submitted by
Milkey Mouse
Severity
normal
M
M
Milkey Mouse wrote on 19 May 2021 20:37
(address . guix-patches@gnu.org)
20210519183749.32709-1-milkeymouse@meme.institute
* gnu/packages/rust.scm (rust-1.49)[source]: Add patch.
[phases]{unpack-profiler-rt, enable-profiling}: New phases.
[native-inputs]{compiler-rt-source}: New input.
(rust-1.50)[source]: Remove "rust-1.49-llvm-cov-no-debug.patch".
* gnu/packages/patches/rust-1.49-llvm-cov-no-debug.patch: New file.
* gnu/local.mk (dist_patch_DATA): Add patch.
---
gnu/local.mk | 1 +
.../patches/rust-1.49-llvm-cov-no-debug.patch | 27 +++++++++++++++
gnu/packages/rust.scm | 33 +++++++++++++++++--
3 files changed, 58 insertions(+), 3 deletions(-)
create mode 100644 gnu/packages/patches/rust-1.49-llvm-cov-no-debug.patch

Toggle diff (118 lines)
diff --git a/gnu/local.mk b/gnu/local.mk
index add01cf6d2..9dbf568d0c 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -1631,6 +1631,7 @@ dist_patch_DATA = \
%D%/packages/patches/rnp-disable-ruby-rnp-tests.patch \
%D%/packages/patches/rnp-unbundle-googletest.patch \
%D%/packages/patches/ruby-sanitize-system-libxml.patch \
+ %D%/packages/patches/rust-1.49-llvm-cov-no-debug.patch \
%D%/packages/patches/rust-coresimd-doctest.patch \
%D%/packages/patches/rust-ndarray-remove-blas-src-dep.patch \
%D%/packages/patches/rust-nettle-disable-vendor.patch \
diff --git a/gnu/packages/patches/rust-1.49-llvm-cov-no-debug.patch b/gnu/packages/patches/rust-1.49-llvm-cov-no-debug.patch
new file mode 100644
index 0000000000..b72402a9a7
--- /dev/null
+++ b/gnu/packages/patches/rust-1.49-llvm-cov-no-debug.patch
@@ -0,0 +1,27 @@
+cherry-picked from commit fe56d267cae, in master as of 1.50
+diff --git a/src/test/run-make-fulldeps/coverage-reports-base/Makefile b/src/test/run-make-fulldeps/coverage-reports-base/Makefile
+index b175768e199..1e2aa056e40 100644
+--- a/src/test/run-make-fulldeps/coverage-reports-base/Makefile
++++ b/src/test/run-make-fulldeps/coverage-reports-base/Makefile
+@@ -13,15 +13,13 @@
+ BASEDIR=../coverage-reports-base
+ SOURCEDIR=../coverage
+
+-ifeq ($(UNAME),Darwin)
+-# FIXME(richkadel): It appears that --debug is not available on MacOS even when not running
+-# under CI.
+-NO_LLVM_ASSERTIONS=1
+-endif
+-
+ # The `llvm-cov show` flag `--debug`, used to generate the `counters` output files, is only enabled
+-# if LLVM assertions are enabled. Some CI builds disable debug assertions.
+-ifndef NO_LLVM_ASSERTIONS
++# if LLVM assertions are enabled. Requires Rust config `llvm/optimize` and not
++# `llvm/release_debuginfo`. Note that some CI builds disable debug assertions (by setting
++# `NO_LLVM_ASSERTIONS=1`), so it is not OK to fail the test, but `bless`ed test results cannot be
++# generated without debug assertions.
++LLVM_COV_DEBUG := $(shell "$(LLVM_BIN_DIR)"/llvm-cov show --debug 2>&1 | grep -q "Unknown command line argument '--debug'"; echo $$?)
++ifeq ($(LLVM_COV_DEBUG), 1)
+ DEBUG_FLAG=--debug
+ endif
+
diff --git a/gnu/packages/rust.scm b/gnu/packages/rust.scm
index 636420e25d..72355fa789 100644
--- a/gnu/packages/rust.scm
+++ b/gnu/packages/rust.scm
@@ -730,6 +730,10 @@ safety and thread safety guarantees.")
"0yf7kll517398dgqsr7m3gldzj0iwsp3ggzxrayckpqzvylfy2mm")))
(package
(inherit base-rust)
+ (source
+ (origin
+ (inherit (package-source base-rust))
+ (patches (search-patches "rust-1.49-llvm-cov-no-debug.patch"))))
(outputs (cons "doc" (package-outputs base-rust)))
(arguments
(substitute-keyword-arguments (package-arguments base-rust)
@@ -817,11 +821,24 @@ safety and thread safety guarantees.")
((file) file))
(("fn ctrl_c_kills_everyone")
"#[ignore]\nfn ctrl_c_kills_everyone"))))
+ (add-after 'unpack 'unpack-profiler-rt
+ ;; Copy compiler-rt sources to where libprofiler_builtins
+ ;; looks for its vendored copy.
+ (lambda* (#:key inputs #:allow-other-keys)
+ (mkdir-p "src/llvm-project/compiler-rt")
+ (invoke "tar" "-xf" (assoc-ref inputs "compiler-rt-source")
+ "-C" "src/llvm-project/compiler-rt" "--strip-components=1")
+ #t))
(add-after 'configure 'enable-docs
(lambda _
(substitute* "config.toml"
(("docs = false")
"docs = true"))))
+ (add-after 'enable-codegen-tests 'enable-profiling
+ (lambda _
+ (substitute* "config.toml"
+ (("^profiler =.*$") "")
+ (("[[]build[]]") "\n[build]\nprofiler = true\n"))))
(add-after 'configure 'add-gdb-to-config
(lambda* (#:key inputs #:allow-other-keys)
(let ((gdb (assoc-ref inputs "gdb")))
@@ -829,17 +846,27 @@ safety and thread safety guarantees.")
(("^python =.*" all)
(string-append all
"gdb = \"" gdb "/bin/gdb\"\n"))))))))))
- ;; Add test inputs.
(native-inputs (cons*
+ ;; Add test inputs.
;; The tests fail when using GDB 10 (see:
;; https://github.com/rust-lang/rust/issues/79009).
`("gdb" ,gdb-9.2)
`("procps" ,procps)
+ ;; Add compiler-rt-source for libprofiler_builtins,
+ ;; which typically vendors compiler-rt. Needed for PGO.
+ `("compiler-rt-source" ,(package-source clang-runtime-11))
(package-native-inputs base-rust))))))
(define-public rust-1.50
- (rust-bootstrapped-package rust-1.49 "1.50.0"
- "0pjs7j62maiyvkmhp9zrxl528g2n0fphp4rq6ap7aqdv0a6qz5wm"))
+ (let ((base-rust (rust-bootstrapped-package
+ rust-1.49 "1.50.0"
+ "0pjs7j62maiyvkmhp9zrxl528g2n0fphp4rq6ap7aqdv0a6qz5wm")))
+ (package
+ (inherit base-rust)
+ (source
+ (origin
+ (inherit (package-source base-rust))
+ (patches '()))))))
(define-public rust-1.51
(rust-bootstrapped-package rust-1.50 "1.51.0"
--
2.31.1
M
M
Marius Bakke wrote on 23 May 2021 16:20
87bl91bkgf.fsf@gnu.org
Milkey Mouse <milkeymouse@meme.institute> skriver:

Toggle quote (7 lines)
> * gnu/packages/rust.scm (rust-1.49)[source]: Add patch.
> [phases]{unpack-profiler-rt, enable-profiling}: New phases.
> [native-inputs]{compiler-rt-source}: New input.
> (rust-1.50)[source]: Remove "rust-1.49-llvm-cov-no-debug.patch".
> * gnu/packages/patches/rust-1.49-llvm-cov-no-debug.patch: New file.
> * gnu/local.mk (dist_patch_DATA): Add patch.

[...]

Toggle quote (8 lines)
> diff --git a/gnu/packages/patches/rust-1.49-llvm-cov-no-debug.patch b/gnu/packages/patches/rust-1.49-llvm-cov-no-debug.patch
> new file mode 100644
> index 0000000000..b72402a9a7
> --- /dev/null
> +++ b/gnu/packages/patches/rust-1.49-llvm-cov-no-debug.patch
> @@ -0,0 +1,27 @@
> +cherry-picked from commit fe56d267cae, in master as of 1.50

Instead of backporting this patch to 1.49, I think it would be better to
switch the default Rust to the latest version before enabling profiling
support.

Unfortunately 1.50 currently fails to build on core-updates, so we'll
need a volunteer to fix that. Would you like to give it a try? :-)
-----BEGIN PGP SIGNATURE-----

iIUEARYKAC0WIQRNTknu3zbaMQ2ddzTocYulkRQQdwUCYKpksA8cbWFyaXVzQGdu
dS5vcmcACgkQ6HGLpZEUEHfjDwD/RLbauQKzOwJx0bqD+ReypmnhPs+H0RbOAzSB
+ksx+mAA+wcOpqv+eA7sQCJdPowz2CXz8RGzdmErIDLhUcyMcGwJ
=JA9b
-----END PGP SIGNATURE-----

M
M
Milkey Mouse wrote on 25 May 2021 03:13
CBLXF76ODQ34.3FK3HIMXDW3C1@jupiter
On Sun May 23, 2021 at 7:20 AM PDT, Marius Bakke wrote:
Toggle quote (4 lines)
> Instead of backporting this patch to 1.49, I think it would be better to
> switch the default Rust to the latest version before enabling profiling
> support.

OK, I will submit a v2 of the patch. Should I do so as a reply, or
close this thread by emailing the @debbugs.gnu.org address? I'm still a
little new to email- and patch-based workflows.

Toggle quote (4 lines)
>
> Unfortunately 1.50 currently fails to build on core-updates, so we'll
> need a volunteer to fix that. Would you like to give it a try? :-)

rust-1.50? It builds fine for me. I am on commit 42162c84dc (with this
patch on top):

$ ./pre-inst-env guix build rust@1.50
/gnu/store/klr7jrzxl3mzprd88f857kw5mam15h78-rust-1.50.0-cargo
/gnu/store/3x656k3s66kjb87aphly9p5v401af863-rust-1.50.0-doc
/gnu/store/nkkfvyl52mmvx12ccwvs54c44yns4yk5-rust-1.50.0
/gnu/store/kq4i7q55gxfl47124vfkiqzp5bi7737r-rust-1.50.0-rustfmt

I suppose I should note I don't use substitutes--but that shouldn't
make a difference, should it?
M
M
Marius Bakke wrote on 25 May 2021 22:35
87lf82pn5k.fsf@gnu.org
"Milkey Mouse" <milkeymouse@meme.institute> skriver:

Toggle quote (9 lines)
> On Sun May 23, 2021 at 7:20 AM PDT, Marius Bakke wrote:
>> Instead of backporting this patch to 1.49, I think it would be better to
>> switch the default Rust to the latest version before enabling profiling
>> support.
>
> OK, I will submit a v2 of the patch. Should I do so as a reply, or
> close this thread by emailing the @debbugs.gnu.org address? I'm still a
> little new to email- and patch-based workflows.

Simply replying to this issue is great.

Toggle quote (15 lines)
>> Unfortunately 1.50 currently fails to build on core-updates, so we'll
>> need a volunteer to fix that. Would you like to give it a try? :-)
>
> rust-1.50? It builds fine for me. I am on commit 42162c84dc (with this
> patch on top):
>
> $ ./pre-inst-env guix build rust@1.50
> /gnu/store/klr7jrzxl3mzprd88f857kw5mam15h78-rust-1.50.0-cargo
> /gnu/store/3x656k3s66kjb87aphly9p5v401af863-rust-1.50.0-doc
> /gnu/store/nkkfvyl52mmvx12ccwvs54c44yns4yk5-rust-1.50.0
> /gnu/store/kq4i7q55gxfl47124vfkiqzp5bi7737r-rust-1.50.0-rustfmt
>
> I suppose I should note I don't use substitutes--but that shouldn't
> make a difference, should it?

Oh you're right, it does work! Sorry for the confusion. It must have
been a transient failure on my end last I tried it. Substitutes should
not make a difference -- the end result should (ideally) be
bit-identical in either case.

I suppose we can promote Rust 1.52 to the default unless there are
big(?) compatibility problems. IIRC the upcoming librsvg requires it.

Thanks!
Marius
-----BEGIN PGP SIGNATURE-----

iIUEARYKAC0WIQRNTknu3zbaMQ2ddzTocYulkRQQdwUCYK1fhw8cbWFyaXVzQGdu
dS5vcmcACgkQ6HGLpZEUEHdjNQEA6156q6ZEZ8agx48Im288Yl4v5GbGPl110jts
rXv9sbIA/0OSUXhgFQp+I8c99R3KAC357Lh/o2W24YVA3Jl5xRUO
=I4ZO
-----END PGP SIGNATURE-----

M
M
Milkey Mouse wrote on 26 May 2021 01:03
[PATCH core-updates 1/2] gnu: rust: Update to 1.52.
(address . guix-patches@gnu.org)
20210525230350.542603-1-milkeymouse@meme.institute
* gnu/packages/rust.scm (rust): Change to 1.52.
---
gnu/packages/rust.scm | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

Toggle diff (25 lines)
diff --git a/gnu/packages/rust.scm b/gnu/packages/rust.scm
index 2d29c2acd7..6b769bfab2 100644
--- a/gnu/packages/rust.scm
+++ b/gnu/packages/rust.scm
@@ -846,9 +846,9 @@ safety and thread safety guarantees.")
"0ixqkqglv3isxbvl4ldr4byrkx692wghsz3fasy1pn5kr2prnsvs"))
(define-public rust-1.52
- (let ((base-rust
- (rust-bootstrapped-package rust-1.51 "1.52.1"
- "165zs3xzp9dravybwslqs1qhn35agp6wacmzpymqg3qfdni26vrs")))
+ (let ((base-rust (rust-bootstrapped-package
+ rust-1.51 "1.52.1"
+ "165zs3xzp9dravybwslqs1qhn35agp6wacmzpymqg3qfdni26vrs")))
(package
(inherit base-rust)
(inputs
@@ -859,4 +859,4 @@ safety and thread safety guarantees.")
;;; intermediate rusts are built for bootstrapping purposes and should not
;;; be relied upon. This is to ease maintenance and reduce the time
;;; required to build the full Rust bootstrap chain.
-(define-public rust rust-1.49)
+(define-public rust rust-1.52)
--
2.31.1
M
M
Milkey Mouse wrote on 26 May 2021 01:03
[PATCH core-updates 2/2] gnu: rust: Add profiling support.
(address . guix-patches@gnu.org)
20210525230350.542603-2-milkeymouse@meme.institute
* gnu/packages/rust.scm (rust-1.52)
[phases]{unpack-profiler-rt, enable-profiling}: New phases.
[native-inputs]{compiler-rt-source}: New input.
---
gnu/packages/rust.scm | 24 +++++++++++++++++++++++-
1 file changed, 23 insertions(+), 1 deletion(-)

Toggle diff (37 lines)
diff --git a/gnu/packages/rust.scm b/gnu/packages/rust.scm
index 6b769bfab2..7b77d791d1 100644
--- a/gnu/packages/rust.scm
+++ b/gnu/packages/rust.scm
@@ -853,7 +853,29 @@ safety and thread safety guarantees.")
(inherit base-rust)
(inputs
(alist-replace "llvm" (list llvm-12)
- (package-inputs base-rust))))))
+ (package-inputs base-rust)))
+ ;; Add compiler-rt-source for libprofiler_builtins (needed for profiling
+ ;; support), which normally vendors its own copy of compiler-rt.
+ (native-inputs (cons*
+ `("compiler-rt-source" ,(package-source clang-runtime-12))
+ (package-native-inputs base-rust)))
+ (arguments
+ (substitute-keyword-arguments (package-arguments base-rust)
+ ((#:phases phases)
+ `(modify-phases ,phases
+ (add-after 'unpack 'unpack-profiler-rt
+ ;; Copy compiler-rt sources to where libprofiler_builtins
+ ;; looks for its vendored copy.
+ (lambda* (#:key inputs #:allow-other-keys)
+ (mkdir-p "src/llvm-project/compiler-rt")
+ (invoke "tar" "-xf" (assoc-ref inputs "compiler-rt-source")
+ "-C" "src/llvm-project/compiler-rt" "--strip-components=1")
+ #t))
+ (add-after 'enable-codegen-tests 'enable-profiling
+ (lambda _
+ (substitute* "config.toml"
+ (("^profiler =.*$") "")
+ (("[[]build[]]") "\n[build]\nprofiler = true\n")))))))))))
;;; Note: Only the latest versions of Rust are supported and tested. The
;;; intermediate rusts are built for bootstrapping purposes and should not
--
2.31.1
M
M
Maxim Cournoyer wrote on 20 Jan 23:42 +0100
Re: bug#48525: [PATCH core-updates] gnu: rust: Add profiling support.
(name . Milkey Mouse)(address . milkeymouse@meme.institute)
871qabd3tg.fsf_-_@gmail.com
Hi,

Milkey Mouse <milkeymouse@meme.institute> writes:

Toggle quote (43 lines)
> * gnu/packages/rust.scm (rust-1.52)
> [phases]{unpack-profiler-rt, enable-profiling}: New phases.
> [native-inputs]{compiler-rt-source}: New input.
> ---
> gnu/packages/rust.scm | 24 +++++++++++++++++++++++-
> 1 file changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/gnu/packages/rust.scm b/gnu/packages/rust.scm
> index 6b769bfab2..7b77d791d1 100644
> --- a/gnu/packages/rust.scm
> +++ b/gnu/packages/rust.scm
> @@ -853,7 +853,29 @@ safety and thread safety guarantees.")
> (inherit base-rust)
> (inputs
> (alist-replace "llvm" (list llvm-12)
> - (package-inputs base-rust))))))
> + (package-inputs base-rust)))
> + ;; Add compiler-rt-source for libprofiler_builtins (needed for profiling
> + ;; support), which normally vendors its own copy of compiler-rt.
> + (native-inputs (cons*
> + `("compiler-rt-source" ,(package-source clang-runtime-12))
> + (package-native-inputs base-rust)))
> + (arguments
> + (substitute-keyword-arguments (package-arguments base-rust)
> + ((#:phases phases)
> + `(modify-phases ,phases
> + (add-after 'unpack 'unpack-profiler-rt
> + ;; Copy compiler-rt sources to where libprofiler_builtins
> + ;; looks for its vendored copy.
> + (lambda* (#:key inputs #:allow-other-keys)
> + (mkdir-p "src/llvm-project/compiler-rt")
> + (invoke "tar" "-xf" (assoc-ref inputs "compiler-rt-source")
> + "-C" "src/llvm-project/compiler-rt" "--strip-components=1")
> + #t))
> + (add-after 'enable-codegen-tests 'enable-profiling
> + (lambda _
> + (substitute* "config.toml"
> + (("^profiler =.*$") "")
> + (("[[]build[]]") "\n[build]\nprofiler = true\n")))))))))))
>
> ;;; Note: Only the latest versions of Rust are supported and tested. The
> ;;; intermediate rusts are built for bootstrapping purposes and should not

I've ported this change to our current rust (using clang-runtime-15 to
match the llvm version used) and added it to the queued changes I'll
push shortly.

--
Thanks,
Maxim
Closed
?
Your comment

This issue is archived.

To comment on this conversation send an email to 48525@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 48525
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch