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

bug-1884718: update stackwalker to v0.21.1 #14

Merged
merged 1 commit into from Mar 11, 2024
Merged

bug-1884718: update stackwalker to v0.21.1 #14

merged 1 commit into from Mar 11, 2024

Conversation

relud
Copy link
Member

@relud relud commented Mar 11, 2024

This also adds python3-venv to the docker config for running regression tests in docker, and adds a comment to the readme about what permissions are needed on the api key to run regression tests.

This picks up the following changes, in particular 3e18614 Use framehop and wholesym rather than symbolic for the local debuginfo symbol provider. is expected to provide some performance improvement:

e7ecb41 (HEAD -> main, tag: v0.21.1, origin/main, origin/HEAD) chore: Release
dff1530 Updated the release notes for the next version
b65c81a Updated all the dependencies
10eefc4 Merge pull request #968 from afranchuk/dont-require-system-info
73097b4 Only require the MinidumpSystemInfo stream when using local debug info.
2611865 (tag: v0.21.0) chore: Release
2f56803 Updated cargo-dist configuration using version 0.11.1
c154dbd Updated the release notes for the next version
52ffeda Updated all the dependencies
58a239c Allow passing the path to a symbol files in --symbols-path
7b28df6 Merge pull request #963 from rust-minidump/dependabot/cargo/framehop-0.9.0
9030821 Merge pull request #955 from lissyx/get-soname
936ca21 Bump framehop from 0.8.0 to 0.9.0
a66b5c5 Merge pull request #957 from rust-minidump/dependabot/cargo/cachemap2-0.3.0
e4c370c Bump cachemap2 from 0.2.0 to 0.3.0
2cf996d Merge pull request #958 from rust-minidump/dependabot/cargo/num-derive-0.4.2
15a3020 Bump num-derive from 0.4.1 to 0.4.2
74c8656 Merge pull request #952 from rust-minidump/dependabot/cargo/time-0.3.34
3649152 Merge pull request #950 from rust-minidump/dependabot/cargo/env_logger-0.11.1
f33622a Merge pull request #948 from rust-minidump/dependabot/cargo/num-traits-0.2.18
bd1959b Bug 1847098 - Parse version number for Elf files
19931a1 Bump num-traits from 0.2.17 to 0.2.18
d4f6c7f Allow compile-time and runtime selection of symbolication in the DebugInfoSymbolProvider.
b5c57e8 Use a trait object to abstract over framehop platforms.
3e18614 Use framehop and wholesym rather than symbolic for the local debuginfo symbol provider.
6714a70 Remove redundant imports
556ebab Bump clap from 4.4.18 to 4.5.0
b66d682 Bump cab from 0.4.1 to 0.5.0
b5d23e8 clippy
4221d28 parse threadinfolist stream
21673cb Validate `stack_memory` once in `walk_stack`
0289d14 Bump time from 0.3.31 to 0.3.34
fe29edb Bump env_logger from 0.10.2 to 0.11.1
a708dec Get `MinidumpModule` before constructing `CfiStackWalker`
631f649 Reduce duplication a bit more
e0e680c Use a constructor for `CfiStackWalker`
5127da5 Finish refactoring all the args
bb2c07d Turn `args` into a ref and thread it through more fns
f87387e Combine all the arguments into a struct
9d1166b Move `stack_memory` unwrapping one up
88153c0 Unwrap `stack_memory` in the outer `get_caller_frame`
78f52e6 Actually just remove the whole `Unwind` trait
a8ebe53 Use AFIT for the `Unwind` trait
34019f0 Fix CI badge
7146857 Remove unnecessary chrono dependency

Regression tests:

$ ./bin/regression_stats.py regr/20240311_100951 regr/20240311_095115
 crashid                               run1 time  cache size  output size  run2 time  cache size  output size 
 df0c77dd-0d03-4571-a11a-2621f0240301  6          665,676     734,073      13         665,676     734,073     
 5ef6a4c0-19c8-4047-8a4f-7a4f70240301  6          75,776      612,404      7          75,776      612,414     
 b855aacd-2614-47ff-999e-4b9640240301  13         634,976     215,505      19         634,976     215,505     
 d2733453-a1b2-4df0-a88f-349250240301  2          7,588       370,726      2          7,588       370,726     
 d67ba83f-d9a7-40d6-a73a-76c750240301  13         587,464     688,547      15         587,464     688,547     
 81f1cb9e-4498-405a-86b9-e91650240301  15         634,972     188,949      13         634,972     188,949     
 a6cd79a7-d647-4515-84e7-0f4fa0240301  2          8,700       541,228      1          8,700       541,228     
 dbd110bd-871b-44a7-acbd-8eb040240301  3          223,268     177,421      4          223,268     177,421     
 05b9b736-9d69-46ad-a6d7-681720240301  5          693,900     377,728      13         693,900     377,728     
 9d0c08ed-a80c-47b4-b377-65d1e0240301  14         587,464     510,490      15         587,464     510,490     
 1e3b2e71-c2f1-4389-9912-098970240301  15         695,640     735,198      13         695,644     735,198     
 8b1e68bb-0901-46fb-99db-725490240301  12         615,060     467,628      21         615,060     467,628     
 e3bcf8dd-8395-48a8-bfbb-bda550240301  2          123,252     379,324      8          123,256     379,324     
 a8444796-8557-4b92-9364-4d5ac0240301  12         585,788     410,990      23         585,792     410,990     
 cfad8999-bda3-4230-9e6e-f9fda0240301  13         605,248     419,056      18         605,248     419,056     
 1c990f1b-ffdb-4fd9-ba27-9ae890240301  5          587,596     505,254      13         587,600     505,254     
 adb621cd-17fa-46df-85f0-c9cf60240301  5          565,720     508,899      5          565,720     508,899     
 8f1c3949-02b0-4e21-8324-345f30240301  6          783,512     738,186      6          783,504     738,186     
 692715a9-584d-42c4-b7db-a94e70240301  2          8,744       540,681      2          8,744       540,681     
 b9f817a2-13c6-4c7f-bb37-6790d0240301  12         588,688     682,558      5          588,688     682,558

the cache and output sizes are unchanged, and the run times mostly look similar, but 1c990f1b-ffdb-4fd9-ba27-9ae890240301 looked notably slower, so I reran it, which got me times of 22, 11, 15, 17 and 13, so the slowdown seems consistent.

In addition to the crashes in the regression test report, I also tested the error crash from rust-minidump/rust-minidump#967, and both before and after the update it returned:

nocache (1/5) ... stackwalker error: ProcessError('stackwalker: stdout:  stderr: ERROR MissingThreadList - Error processing dump: The thread list stream was not found\n')

@relud relud requested a review from willkg March 11, 2024 18:03
Copy link
Collaborator

@willkg willkg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants