Make allocation profiling work after exec #342

nsavoire · 2023-11-10T00:58:07Z

What does this PR do?

This PR add the ability to profile allocations even after a process has exec'd into a new binary image.

How do this work ?

ddprof now keeps a unix socket open, unix socket path can be overridden with --socket input option.
Worker accepts connections on this socket in a separate thread and sends ring buffer information upon request.
When a new process is exec'd, library looks up DD_PROFILING_NATIVE_LIB_SOCKET env variable to get socket path and if it exists, connects to it otherwise it spawns a ddprof process, then uses a pipe to synchronize with ddprof start and get socket path, and then connects to it.
By default ddprof creates an abstract socket, that does not exist on the filesystem: \0/tmp/ddprof-<pid>.sock.
Exec following can be disabled by setting env variable DD_PROFILING_NATIVE_ALLOCATION_PROFILING_FOLLOW_EXECS to 0/off/no.

include/ipc.hpp

pr-commenter · 2023-11-10T19:27:57Z

Benchmark results for collatz

Parameters

	Baseline	Candidate
config	baseline	candidate
profiler-version	ddprof 0.15.0+f257ed6c.22893508	ddprof 0.15.0+e3efa971.23283787

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 1 metrics, 0 unstable metrics.

See unchanged results

scenario	Δ mean execution_time
scenario:ddprof -S bench-collatz --preset cpu_only collatz_runner.sh	same

pr-commenter · 2023-11-10T19:27:59Z

Benchmark results for BadBoggleSolver_run

Parameters

	Baseline	Candidate
config	baseline	candidate
profiler-version	ddprof 0.15.0+f257ed6c.22893508	ddprof 0.15.0+e3efa971.23283787

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 1 metrics, 0 unstable metrics.

See unchanged results

scenario	Δ mean execution_time
scenario:ddprof -S bench-bad-boggle-solver BadBoggleSolver_run work 1000	same

r1viollet · 2023-11-13T15:44:59Z

test/simple_malloc-ut.sh

+check "./ddprof bash -c \"./test/simple_malloc ${opts}\"" 1
+
+# Test disabling follow execs
+check "env DD_PROFILING_NATIVE_ALLOCATION_PROFILING_FOLLOW_EXECS=0 DD_PROFILING_NATIVE_STARTUP_WAIT_MS=200 ./ddprof bash -c \"./test/simple_malloc ${opts}\"" 0 0 "inuse-space"


Interesting to test the removal of the feature. For overhead, there might be a use case where users do not want to follow some execs. Good thinking.

r1viollet · 2023-11-13T15:58:17Z

src/ddprof_cli.cc

@@ -297,10 +297,14 @@ int DDProfCLI::parse(int argc, const char *argv[]) {
      app.add_flag("--help_extended", help_extended, "Show extended options")
          ->group(""));
  extended_options.push_back(
-      app.add_option("--socket", socket,
-                     "Profiler's IPC socket, as a file descriptor")
+      app.add_option("--socket", socket_path, "Profiler socket path")


Minor: we could give a more detailed explanation of why we establish this socket.

Do you mean adding details to the help text or adding comments to explain why we need a socket ?
FYI I added an overview of the communication process between profiler/library here.

As a user I would be confused with what I can do with this.
The overview is great.

I guess these are more developer options. We do not use them apart from testing use cases. Would something like:
Override the automatically created socket with a specific path
Be better ?

ddprof now keeps a unix socket open, unix socket path can be overridden with --socket input option. Worker accepts connections on this socket in a separate thread and sends ring buffer information upon request. When a new process is exec'd, library looks up DD_PROFILING_NATIVE_LIB_SOCKET env variable to get socket path and if it exists, connects to it otherwise it spawns a ddprof process, then uses a pipe to synchronize with ddprof start and get socket path, and then connects to it. By default ddprof creates an abstract socket, that does not exist on the filesystem: \0/tmp/ddprof-<pid>.sock. This behaviour can be disabled by setting env variable DD_PROFILING_NATIVE_ALLOCATION_PROFILING_FOLLOW_EXECS to 0/off/no.

src/lib/dd_profiling.cc

src/exe/main.cc

r1viollet · 2023-11-14T08:12:23Z

src/perf_mainloop.cc

@@ -109,6 +111,45 @@ DDRes spawn_workers(PersistentWorkerState *persistent_worker_state,
  return {};
 }

+ReplyMessage create_reply_message(const DDProfContext &ctx) {


minor: it feels slightly weird to have this function defined here.

r1viollet · 2023-11-14T08:13:31Z

src/perf_mainloop.cc

+
+      if (ctx.watchers[alloc_watcher_idx].aggregation_mode ==
+          EventAggregationMode::kLiveSum) {
+        reply.allocation_flags |= (1 << ReplyMessage::kLiveCallgraph);


it seems someone forgot to rename the options all the way down 😅

r1viollet · 2023-11-14T08:15:29Z

src/perf_mainloop.cc

  int pfd_len = 0;
  pollfd_setup(&ctx.worker_ctx.pevent_hdr, poll_fds, &pfd_len);

  DDRES_CHECK_FWD(signalfd_setup(&poll_fds[pfd_len]));
  int const signal_pos = pfd_len++;

+  WorkerServer const server =


Somehow I had not thought about making this a separate thread. I thought we would make it live within the mainloop.
I think it is better this way.

r1viollet

LGTM
A very impressive capability.
I am eager to start playing with this.

r1viollet

LGTM

nsavoire requested review from sanchda and r1viollet as code owners November 10, 2023 00:58

r1viollet reviewed Nov 10, 2023

View reviewed changes

include/ipc.hpp Show resolved Hide resolved

nsavoire force-pushed the nsavoire/follow_execs branch from b7b6411 to a95dfb7 Compare November 13, 2023 12:22

nsavoire added 6 commits November 13, 2023 13:39

Add missing nullptr at the end of arg array

982d6af

Use c++20 std::jthread

c497db2

Return pipe betwen daemon -> initial process as part of DaemonizeResult

799a727

Add xoshiro256** prng

a2fafa1

Modernize arg_which and replace arg_yesno by arg_yes and arg_no

ba617af

Add exec_defer to allow early execution of scope_exit

da01690

nsavoire force-pushed the nsavoire/follow_execs branch from a95dfb7 to 6a91d73 Compare November 13, 2023 14:04

r1viollet reviewed Nov 13, 2023

View reviewed changes

nsavoire added 3 commits November 13, 2023 17:00

Print whole input command line as an array in show_config

fbfbf55

Add tests for following execs for allocation profiling

d57d94b

nsavoire force-pushed the nsavoire/follow_execs branch from 6a91d73 to d57d94b Compare November 13, 2023 17:00

r1viollet reviewed Nov 14, 2023

View reviewed changes

src/lib/dd_profiling.cc Outdated Show resolved Hide resolved

r1viollet reviewed Nov 14, 2023

View reviewed changes

src/exe/main.cc Show resolved Hide resolved

r1viollet reviewed Nov 14, 2023

View reviewed changes

r1viollet previously approved these changes Nov 14, 2023

View reviewed changes

nsavoire added 2 commits November 14, 2023 09:20

Move IPC architecture description to design doc

02f53d2

Improve socket option help

865407b

nsavoire dismissed r1viollet’s stale review via 865407b November 14, 2023 09:20

Update allocation flag naming

e3efa97

nsavoire requested a review from r1viollet November 14, 2023 13:24

r1viollet approved these changes Nov 14, 2023

View reviewed changes

nsavoire merged commit 0b28374 into main Nov 14, 2023

nsavoire deleted the nsavoire/follow_execs branch November 14, 2023 14:52

nsavoire mentioned this pull request Feb 16, 2024

Follow execs with allocation profiling #182

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make allocation profiling work after exec #342

Make allocation profiling work after exec #342

nsavoire commented Nov 10, 2023 •

edited

Loading

pr-commenter bot commented Nov 10, 2023 •

edited

Loading

pr-commenter bot commented Nov 10, 2023 •

edited

Loading

r1viollet Nov 13, 2023

r1viollet Nov 13, 2023

nsavoire Nov 13, 2023 •

edited

Loading

r1viollet Nov 14, 2023

r1viollet Nov 14, 2023

r1viollet Nov 14, 2023

r1viollet Nov 14, 2023

r1viollet Nov 14, 2023

r1viollet left a comment

r1viollet left a comment

Make allocation profiling work after exec #342

Make allocation profiling work after exec #342

Conversation

nsavoire commented Nov 10, 2023 • edited Loading

What does this PR do?

How do this work ?

pr-commenter bot commented Nov 10, 2023 • edited Loading

Benchmark results for collatz

Parameters

Summary

pr-commenter bot commented Nov 10, 2023 • edited Loading

Benchmark results for BadBoggleSolver_run

Parameters

Summary

r1viollet Nov 13, 2023

Choose a reason for hiding this comment

r1viollet Nov 13, 2023

Choose a reason for hiding this comment

nsavoire Nov 13, 2023 • edited Loading

Choose a reason for hiding this comment

r1viollet Nov 14, 2023

Choose a reason for hiding this comment

r1viollet Nov 14, 2023

Choose a reason for hiding this comment

r1viollet Nov 14, 2023

Choose a reason for hiding this comment

r1viollet Nov 14, 2023

Choose a reason for hiding this comment

r1viollet Nov 14, 2023

Choose a reason for hiding this comment

r1viollet left a comment

Choose a reason for hiding this comment

r1viollet left a comment

Choose a reason for hiding this comment

nsavoire commented Nov 10, 2023 •

edited

Loading

pr-commenter bot commented Nov 10, 2023 •

edited

Loading

pr-commenter bot commented Nov 10, 2023 •

edited

Loading

nsavoire Nov 13, 2023 •

edited

Loading