[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
?