Granulate / gprofiler

gProfiler is a system-wide profiler, combining multiple sampling profilers to produce unified visualization of what your CPU is spending time on.
https://profiler.granulate.io
Apache License 2.0
757 stars 55 forks source link

java: Upgrade to async-profiler v2.9 #596

Closed Jongy closed 1 year ago

Jongy commented 1 year ago

This resolves https://github.com/Granulate/gprofiler/issues/563 because it was fixed by https://github.com/jvm-profiling-tools/async-profiler/commit/b0a44524bad111a7f18c8309c0ebec56ccfdcf2a which is included.

EDIT 29th Nov: After upgrading to 2.9, our CentOS 6 broke. We've had a old hack which forced linking with memcpy@GLIBC_2.2.5, and it fails on 2.9, from bisecting I found the breaking commit to be https://github.com/jvm-profiling-tools/async-profiler/commit/b0a44524bad111a7f18c8309c0ebec56ccfdcf2a which, I figure, introduced lots of implicit string handling which added calls to memcpy that were not affected by the symbol version pinning.

Soooo after lots of trying around of different .symver directives and other hacks such as directly passing lib.so of glibc-2.12 in the build command line (which causes gcc to take symbol versions from that DSO, but it might have adverse effects if I built against headers of version X then linked against incompatible version Y...)

I ended up -

Jongy commented 1 year ago
$ git range-diff upstream/master..v2.8.3g5 upstream/master..v2.9g1
 1:  f03014d =  1:  f2fd456 Replace semicolon in method descriptors to vertical bar, since a semicolon is already used to separate between methods
 2:  56daea2 =  2:  f611400 Remove the "Profiling started" message
 3:  e406f18 =  3:  63e3017 Remove the "Profiling stopped" message
 4:  7a4312d =  4:  b367d20 Compile statically with libstdc++
 5:  52692de =  5:  2ae2635 Build jattach statically
 6:  e6abbed =  6:  f4b205d Build fdtransfer statically
 7:  4699370 =  7:  6c034cc Compile statically with libgcc
 8:  1bef4f5 =  8:  866c156 Force link with memcpy@GLIBC_2.2.5 on x86_64
 9:  4018ebb =  9:  203495e Fix -static-* args passed only in non-MERGE case :(
10:  ee5acc1 ! 10:  f9b9fda Close standard streams after forking in fdtransfer
    @@ Commit message
         Close standard streams after forking in fdtransfer

      ## src/fdtransfer/fdtransferServer.cpp ##
    -@@ src/fdtransfer/fdtransferServer.cpp: static int single_pid_server(int pid) {
    +@@ src/fdtransfer/fdtransferServer.cpp: static int single_pid_server(int pid, const char *path) {

          // CLONE_NEWPID affects children only - so we fork here.
          if (0 == fork()) {
11:  1eed7a1 <  -:  ------- Generate randomized fdtransfer path per async-profiler invocation (#635)
12:  dcfe236 = 11:  1102ecc -static-libstdc++ only on musl
13:  7b51a01 <  -:  ------- Do not use ELF NODELETE flag because of glibc bug
14:  da08aab ! 12:  aa4669c Convert frame name format to 'function (DSO)' for LIB style (#2)
    @@ Commit message
         For https://github.com/Granulate/gprofiler/pull/491

      ## src/codeCache.cpp ##
    -@@ src/codeCache.cpp: void NativeFunc::destroy(char* name) {
    +@@ src/codeCache.cpp: size_t NativeFunc::usedMemory(const char* name) {

      CodeCache::CodeCache(const char* name, short lib_index, const void* min_address, const void* max_address) {
          _name = NativeFunc::create(name, -1);
    @@ src/frameName.cpp: const char* FrameName::decodeNativeSymbol(const char* name) {
              char* demangled = abi::__cxa_demangle(name, NULL, NULL, &status);
              if (demangled != NULL) {
                  if (lib_name != NULL) {
    --                snprintf(_buf, sizeof(_buf) - 1, "%s`%s", lib_name, demangled);
    -+                snprintf(_buf, sizeof(_buf) - 1, "%s (%s)", demangled, lib_name);
    +-                _str.assign(lib_name).append("`").append(demangled);
    ++                _str.assign(demangled).append(" (").append(lib_name).append(")");
                  } else {
    -                 strncpy(_buf, demangled, sizeof(_buf) - 1);
    +                 _str.assign(demangled);
                  }
     @@ src/frameName.cpp: const char* FrameName::decodeNativeSymbol(const char* name) {
          }

          if (lib_name != NULL) {
    --        snprintf(_buf, sizeof(_buf) - 1, "%s`%s", lib_name, name);
    -+        snprintf(_buf, sizeof(_buf) - 1, "%s (%s)", name, lib_name);
    -         return _buf;
    +-        return _str.assign(lib_name).append("`").append(name).c_str();
    ++        return _str.assign(name).append(" (").append(lib_name).append(")").c_str();
          } else {
              return name;
    +     }

      ## src/profiler.cpp ##
     @@ src/profiler.cpp: const char* Profiler::getLibraryName(const char* native_symbol) {
Jongy commented 1 year ago

Looks good

Jongy commented 1 year ago

I'll update to the stable v2.9 release once it happens, and we merge it

Jongy commented 1 year ago

AP v2.9 was released, I did the rebase again. Attaching the range-diff:

$ git range-diff upstream/master..v2.8.3g6 upstream/master..v2.9g1
 1:  f03014d =  1:  6242654 Replace semicolon in method descriptors to vertical bar, since a semicolon is already used to separate between methods
 2:  56daea2 =  2:  35531a4 Remove the "Profiling started" message
 3:  e406f18 =  3:  473b925 Remove the "Profiling stopped" message
 4:  7a4312d =  4:  bacc840 Compile statically with libstdc++
 5:  52692de =  5:  f162e85 Build jattach statically
 6:  e6abbed =  6:  d2d2c12 Build fdtransfer statically
 7:  4699370 =  7:  900bbf0 Compile statically with libgcc
 8:  1bef4f5 =  8:  70773ed Force link with memcpy@GLIBC_2.2.5 on x86_64
 9:  4018ebb =  9:  718346f Fix -static-* args passed only in non-MERGE case :(
10:  ee5acc1 ! 10:  b463b60 Close standard streams after forking in fdtransfer
    @@ Commit message
         Close standard streams after forking in fdtransfer

      ## src/fdtransfer/fdtransferServer.cpp ##
    -@@ src/fdtransfer/fdtransferServer.cpp: static int single_pid_server(int pid) {
    +@@ src/fdtransfer/fdtransferServer.cpp: static int single_pid_server(int pid, const char *path) {

          // CLONE_NEWPID affects children only - so we fork here.
          if (0 == fork()) {
11:  1eed7a1 <  -:  ------- Generate randomized fdtransfer path per async-profiler invocation (#635)
12:  dcfe236 = 11:  a1fc72d -static-libstdc++ only on musl
13:  7b51a01 <  -:  ------- Do not use ELF NODELETE flag because of glibc bug
14:  da08aab ! 12:  59b0ba4 Convert frame name format to 'function (DSO)' for LIB style (#2)
    @@ Commit message
         For https://github.com/Granulate/gprofiler/pull/491

      ## src/codeCache.cpp ##
    -@@ src/codeCache.cpp: void NativeFunc::destroy(char* name) {
    +@@ src/codeCache.cpp: size_t NativeFunc::usedMemory(const char* name) {

      CodeCache::CodeCache(const char* name, short lib_index, const void* min_address, const void* max_address) {
          _name = NativeFunc::create(name, -1);
    @@ src/frameName.cpp: const char* FrameName::decodeNativeSymbol(const char* name) {
              char* demangled = abi::__cxa_demangle(name, NULL, NULL, &status);
              if (demangled != NULL) {
                  if (lib_name != NULL) {
    --                snprintf(_buf, sizeof(_buf) - 1, "%s`%s", lib_name, demangled);
    -+                snprintf(_buf, sizeof(_buf) - 1, "%s (%s)", demangled, lib_name);
    +-                _str.assign(lib_name).append("`").append(demangled);
    ++                _str.assign(demangled).append(" (").append(lib_name).append(")");
                  } else {
    -                 strncpy(_buf, demangled, sizeof(_buf) - 1);
    +                 _str.assign(demangled);
                  }
     @@ src/frameName.cpp: const char* FrameName::decodeNativeSymbol(const char* name) {
          }

          if (lib_name != NULL) {
    --        snprintf(_buf, sizeof(_buf) - 1, "%s`%s", lib_name, name);
    -+        snprintf(_buf, sizeof(_buf) - 1, "%s (%s)", name, lib_name);
    -         return _buf;
    +-        return _str.assign(lib_name).append("`").append(name).c_str();
    ++        return _str.assign(name).append(" (").append(lib_name).append(")").c_str();
          } else {
              return name;
    +     }

      ## src/profiler.cpp ##
     @@ src/profiler.cpp: const char* Profiler::getLibraryName(const char* native_symbol) {
15:  bbacc3c = 13:  d42d8d3 Add accept timeout as 3rd parameter of fdtransfer
Jongy commented 1 year ago

Exe build fails because it now requires GLIBC 2.14 -

 > [async-profiler-centos-min-test-glibc 3/3] RUN if ldd /libasyncProfiler.so 2>&1 | grep -q "not found" ; then echo "libasyncProfiler.so is not compatible with minimum CentOS!"; ldd /libasyncProfiler.so; exit 1; fi:
#97 0.243 libasyncProfiler.so is not compatible with minimum CentOS!
#97 0.244   linux-vdso.so.1 =>  (0x00007ffdcf0b1000)
#97 0.244   libdl.so.2 => /lib64/libdl.so.2 (0x00007f985c00c000)
#97 0.244   libpthread.so.0 => /lib64/libpthread.so.0 (0x00007f985bdef000)
#97 0.244   librt.so.1 => /lib64/librt.so.1 (0x00007f985bbe7000)
#97 0.244   libstdc++.so.6 => /usr/lib64/libstdc++.so.6 (0x00007f985b8e1000)
#97 0.244   libm.so.6 => /lib64/libm.so.6 (0x00007f985b65d000)
#97 0.244   libc.so.6 => /lib64/libc.so.6 (0x00007f985b2c9000)
#97 0.244   /lib64/ld-linux-x86-64.so.2 (0x00007f985c46e000)
#97 0.244   libgcc_s.so.1 => /lib64/libgcc_s.so.1 (0x00007f985b0b3000)
#97 0.244 /libasyncProfiler.so: /lib64/libc.so.6: version `GLIBC_2.14' not found (required by /libasyncProfiler.so)

I'll fix it

Jongy commented 1 year ago

I think it'll pass now but I'm not yet certain if this solution is legit at all

Jongy commented 1 year ago

I took a new approach. Build passes locally, let's see if it tests pass and I will also test Aarch64 build now.

Jongy commented 1 year ago

@guybortnikov ready for review

Jongy commented 1 year ago

Final range-diff (you can review the particular commits that were added/changed)

$ git range-diff upstream/master..v2.8.3g6 upstream/master..v2.9g2
 1:  f03014d =  1:  6242654 Replace semicolon in method descriptors to vertical bar, since a semicolon is already used to separate between methods
 2:  56daea2 =  2:  35531a4 Remove the "Profiling started" message
 3:  e406f18 =  3:  473b925 Remove the "Profiling stopped" message
 4:  7a4312d =  4:  bacc840 Compile statically with libstdc++
 5:  52692de =  5:  f162e85 Build jattach statically
 6:  e6abbed =  6:  d2d2c12 Build fdtransfer statically
 7:  4699370 =  7:  900bbf0 Compile statically with libgcc
 8:  1bef4f5 <  -:  ------- Force link with memcpy@GLIBC_2.2.5 on x86_64
 9:  4018ebb =  8:  54c85f6 Fix -static-* args passed only in non-MERGE case :(
10:  ee5acc1 !  9:  6be6afa Close standard streams after forking in fdtransfer
    @@ Commit message
         Close standard streams after forking in fdtransfer

      ## src/fdtransfer/fdtransferServer.cpp ##
    -@@ src/fdtransfer/fdtransferServer.cpp: static int single_pid_server(int pid) {
    +@@ src/fdtransfer/fdtransferServer.cpp: static int single_pid_server(int pid, const char *path) {

          // CLONE_NEWPID affects children only - so we fork here.
          if (0 == fork()) {
11:  1eed7a1 <  -:  ------- Generate randomized fdtransfer path per async-profiler invocation (#635)
12:  dcfe236 = 10:  0d3e5a6 -static-libstdc++ only on musl
13:  7b51a01 <  -:  ------- Do not use ELF NODELETE flag because of glibc bug
14:  da08aab ! 11:  bb1f846 Convert frame name format to 'function (DSO)' for LIB style (#2)
    @@ Commit message
         For https://github.com/Granulate/gprofiler/pull/491

      ## src/codeCache.cpp ##
    -@@ src/codeCache.cpp: void NativeFunc::destroy(char* name) {
    +@@ src/codeCache.cpp: size_t NativeFunc::usedMemory(const char* name) {

      CodeCache::CodeCache(const char* name, short lib_index, const void* min_address, const void* max_address) {
          _name = NativeFunc::create(name, -1);
    @@ src/frameName.cpp: const char* FrameName::decodeNativeSymbol(const char* name) {
              char* demangled = abi::__cxa_demangle(name, NULL, NULL, &status);
              if (demangled != NULL) {
                  if (lib_name != NULL) {
    --                snprintf(_buf, sizeof(_buf) - 1, "%s`%s", lib_name, demangled);
    -+                snprintf(_buf, sizeof(_buf) - 1, "%s (%s)", demangled, lib_name);
    +-                _str.assign(lib_name).append("`").append(demangled);
    ++                _str.assign(demangled).append(" (").append(lib_name).append(")");
                  } else {
    -                 strncpy(_buf, demangled, sizeof(_buf) - 1);
    +                 _str.assign(demangled);
                  }
     @@ src/frameName.cpp: const char* FrameName::decodeNativeSymbol(const char* name) {
          }

          if (lib_name != NULL) {
    --        snprintf(_buf, sizeof(_buf) - 1, "%s`%s", lib_name, name);
    -+        snprintf(_buf, sizeof(_buf) - 1, "%s (%s)", name, lib_name);
    -         return _buf;
    +-        return _str.assign(lib_name).append("`").append(name).c_str();
    ++        return _str.assign(name).append(" (").append(lib_name).append(")").c_str();
          } else {
              return name;
    +     }

      ## src/profiler.cpp ##
     @@ src/profiler.cpp: const char* Profiler::getLibraryName(const char* native_symbol) {
15:  bbacc3c = 12:  92045e8 Add accept timeout as 3rd parameter of fdtransfer
 -:  ------- > 13:  d761b57 Use compat-glibc on x86_64 glibc
Jongy commented 1 year ago

I reviewed again the AP changes & these changes.