Implement fast dump #2047 #2121

alhah · 2021-05-05T01:32:55Z

All you need to do is to add the dependency of ‘fast-dump’, 'DumpWrapper' will automatically do the rest.
Support Android 5~11.

Signed-off-by: xueqiushi xueqiushi@kuaishou.com

CLAassistant · 2021-05-05T01:32:59Z

All committers have signed the CLA.

All you need to do is to add the dependency of ‘fast-dump’, 'DumpWrapper' will automatically do the rest. Support Android 5~11. Signed-off-by: xueqiushi <xueqiushi@kuaishou.com>

pyricau · 2021-05-11T23:33:31Z

Nit: can you rename the top folder and artifact name from fast-dump to android-fast-dump? Just to make it clear this is an Android specific artifact.

fast-dump/build.gradle

fast-dump/gradle.properties

fast-dump/src/main/AndroidManifest.xml

fast-dump/src/main/cpp/klog.h

pyricau · 2021-05-11T23:46:47Z

fast-dump/src/main/java/com/squareup/leakcanary/FastDump.kt

+    try {
+      System.loadLibrary("fast-dump")
+    } catch (e: UnsatisfiedLinkError ) {
+      e.printStackTrace()


Ideally there should probably an explicit load method, maybe with a boolean indicating whether loading was successfull. Then the consumer can check that before calling forkDumpHprof

That would make the reflection calls a bit more cumbersome, but also allows to fallback to default if it didn't work.

The check & fallback implements in DumpWrapper.dumpHprofData with InvocationTargetException

leakcanary-android-utils/build.gradle

leakcanary-android-utils/src/main/java/leakcanary/DumpWrapper.kt

pyricau · 2021-05-11T23:49:53Z

leakcanary-android-utils/src/main/java/leakcanary/DumpWrapper.kt

+  @Throws(IOException::class)
+  fun dumpHprofData(fileName: String) {
+    if (isSupportFastDump()) {
+      checkNotMainThread()


Why is not main thread important?

forkDumpHprof is a synchronous call，it use waitpid to wait child process to complete dump hprof, and then shark can analyze it. If someone call it in main thread, it will block the user.

ok. The trick is, this is currently exposed as an API on the LeakCanary object so someone might call it from the main thread. I'm not sure what to do, if we should enforce that at the callsite, switch off to a background thread, or just not use fast dump if called from the main thread.

I think fallback in main thread called situation is better. If someone does not realize that fast dump should be called in a background thread, it is hardly for him to use fast dump correctly.
Switch off to a background thread automatically would make someone confused.

Interesting. So what you're saying is that with "fast dump" the dumping isn't faster, but it's non blocking which means the parent process can keep running while the dump is happening in a child process, so it doesn't make sense to wait for that on the main thread and block. And in that case we might as well use the default API. Ok I like it.

pyricau · 2021-05-11T23:50:55Z

leakcanary-android-utils/src/main/java/leakcanary/DumpWrapper.kt

+      try {
+        fastDumpClass?.getDeclaredMethod("forkDumpHprof", String::class.java)?.invoke(null, fileName)
+      } catch (e: Exception) {
+        e.printStackTrace()


What exception do you expect here? We should be specific, and handler better than with default logging. Can you use SharkLog ?

Also the fallback on failure should be just Debug.dumpHprofData no?

Load library fail or something unexpected，add a fallback is a good idea.

Change to forkDumpHprofMethod

pyricau · 2021-05-11T23:51:40Z

leakcanary-android-utils/src/main/java/leakcanary/DumpWrapper.kt

+
+  private val fastDumpClass by lazy {
+    try {
+      return@lazy Class.forName("com.squareup.leakcanary.FastDump")


Yep, this is good, it means fast dump is optional and you have to specifically include the artifact to get it.

Rename the top folder and artifact name from 'fast-dump' to 'android-fast-dump' Signed-off-by: xueqiushi <xueqiushi@kuaishou.com>

Signed-off-by: xueqiushi <xueqiushi@kuaishou.com>

sstallion

Thanks for your contribution @alhah! I'm excited to see native code being contributed to Leak Canary, but since this is the first PR to do so there might be a little more work to get this merged.

In general, your changes look good but I would like to see your CMake list files updated with some additional warning/error checks. Also, would it be possible to update your code to reflect the conventions documented in Google's C++ style guide (See https://google.github.io/styleguide/cppguide.html)?

Thanks again for the hard work!

android-fast-dump/src/main/cpp/CMakeLists.txt

Signed-off-by: xueqiushi <xueqiushi@kuaishou.com>

alhah · 2021-06-29T06:39:56Z

Is there any other problem? @sstallion

pyricau · 2021-06-29T07:42:16Z

I do want to merge this but I'm on paternity leave so not working at the moment.

alhah · 2021-06-29T07:51:17Z

Congratulations, have a good holiday.

pyricau · 2021-09-16T22:59:48Z

I started looking more closely, playing with it and adding a few minor changes.

One thing I'm curious about: when I tried to push to an x86 emulator, of course we don't have a native fast dump lib for it. How can we configure the library so that it's a no-op on non supported architectures, without adb outputting an error:

Execution failed for task ‘:leakcanary-android-sample:installDebug’.
java.util.concurrent.ExecutionException: com.android.builder.testing.api.DeviceException: com.android.ddmlib.InstallException: INSTALL_FAILED_NO_MATCHING_ABIS: Failed to extract native libraries, res=-113

1. Support x86/x86_64 for emulator. 2. Optimize symbol search performance & reliability, support symbol in .gnu_debugdata. 3. Android R fast dump uses another implementation, which is consistent with the lower version. 4. Use c++17 instead of c++11 for make_unique. 5. Workaround for NDK bug: android/ndk#721 Signed-off-by: xueqiushi <xueqiushi@kuaishou.com>

alhah · 2021-09-20T08:42:32Z

I started looking more closely, playing with it and adding a few minor changes.

One thing I'm curious about: when I tried to push to an x86 emulator, of course we don't have a native fast dump lib for it. How can we configure the library so that it's a no-op on non supported architectures, without adb outputting an error:

Execution failed for task ‘:leakcanary-android-sample:installDebug’.
java.util.concurrent.ExecutionException: com.android.builder.testing.api.DeviceException: com.android.ddmlib.InstallException: INSTALL_FAILED_NO_MATCHING_ABIS: Failed to extract native libraries, res=-113

Support x86/x86_64 for emulator and do some optimization

Android R fast dump uses another implementation, which is consistent with the lower version.
"lzma" is a library copied from AOSP to decompress symbol data in ”.gnu_debugdata", so the compile warnings were not handled.

alhah · 2021-12-22T06:27:23Z

For example, dlsym(handle, "xxx") in kwai_dlfcn need to search xxx.so, the information of these symbols are stored in different sections, such as .dynsym, .symtab. However, in the high version of the android system, in order to reduce the library size, the information of the .symtab section will be compressed with 7z and saved in the .gnu_debugdata section when compiling, so it is necessary to decompress it first when searching for symbols.

pyricau · 2021-12-22T16:41:48Z

Thanks!

This PR has a ton of code, and I'm not familiar with the intricacies of C. However, I do want to enable LeakCanary to benefit from these speedups.

As a first step, I'm creating an API to customize the heap dumper in #2237

That way you'll be able to do something like:

LeakCanary.config = LeakCanary.config.copy(
  heapDumper = HeapDumper {
    DumpWrapper.dumpHprofData(it.absolutePath)
  }
}

pyricau · 2022-01-10T18:30:59Z

@alhah hi! With LeakCanary 2.8, it's now possible to customize the heap dumper.

I'm really sorry this PR has been open for so long. It's really hard for me to review it as I'm not comfortable with native code. It would be even harder for me to maintain it in the future, so merging this PR also means increased maintenance burden and higher risk.

I was thinking, an alternate path here would be for you to create your own repo (e.g. android-fast-dump), upload artifacts on maven central directly, and then we can add instructions in the docs on how to use it with LeakCanary?

alhah · 2022-01-11T00:36:46Z

That's OK, it's not hard for me, I can integrate KOOM into the custom dumper.

pyricau · 2022-04-02T16:40:58Z

@alhah Thanks! Do you want to document somewhere (maybe on a Readme in KOOM) how to integrate the two things together? Then I can add a link from the LeakCanary docs to the Koom readme that explains how to use KOOM's heap dump for LeakCanary?

alhah · 2022-04-06T02:22:13Z

We are preparing a new version of KOOM, providing an API to support custom dumper.

alhah · 2022-05-03T13:02:28Z

We just released 2.1.0 to support custom fork dumper, I tried to integrate it into leakcanary, it works, but the code is not very elegant, I will optimize it and release a new version soon.

Here is the patch for test.
forkdump.txt

Here is the logcat, 6757 is the main process id and 6800 is the child process id.

05-03 20:35:08.614 10137  6757  6785 I OOMMonitor_ForkJvmHeapDumper: dump /storage/emulated/0/Download/leakcanary-com.example.leakcanary/2022-05-03_20-35-08_216.hprof
05-03 20:35:08.614 10137  6757  6785 I OOMMonitor_ForkJvmHeapDumper: before suspend and fork.

05-03 20:35:08.620 10137  6800  6800 I mple.leakcanar: hprof: heap dump "/storage/emulated/0/Download/leakcanary-com.example.leakcanary/2022-05-03_20-35-08_216.hprof" starting...
05-03 20:35:10.011 10137  6800  6800 I mple.leakcanar: hprof: heap dump completed (23MB) in 1.390s objects 324186 objects with stack traces 0
05-03 20:35:10.110 10137  6800  6800 I JNIBridge: process 6800 will exit!

05-03 20:35:10.117 10137  6757  6785 I OOMMonitor_ForkJvmHeapDumper: notify from pid 6800

pyricau · 2022-05-03T13:59:46Z

Very cool! When you have the new release, can you open an issue with the setup steps and I'll add it to the documentation?

alhah · 2022-05-04T06:40:19Z

Sure.

alhah · 2022-05-23T16:33:18Z

Very cool! When you have the new release, can you open an issue with the setup steps and I'll add it to the documentation?

Done in #2387. @pyricau

Implement fast dump square#2047

79bbf35

All you need to do is to add the dependency of ‘fast-dump’, 'DumpWrapper' will automatically do the rest. Support Android 5~11. Signed-off-by: xueqiushi <xueqiushi@kuaishou.com>

alhah force-pushed the alhah/fast-dump branch from ec0f170 to 79bbf35 Compare May 5, 2021 02:12

pyricau mentioned this pull request May 11, 2021

Implement fast dump #2047

Closed

pyricau approved these changes May 11, 2021

View reviewed changes

alhah added 2 commits May 12, 2021 07:54

Updatd API changes

cb2ca09

Rename the top folder and artifact name from 'fast-dump' to 'android-fast-dump' Signed-off-by: xueqiushi <xueqiushi@kuaishou.com>

Resovle some comments

a27e3d0

Signed-off-by: xueqiushi <xueqiushi@kuaishou.com>

alhah force-pushed the alhah/fast-dump branch from d9cc254 to a27e3d0 Compare May 12, 2021 14:47

sstallion suggested changes May 12, 2021

View reviewed changes

android-fast-dump/src/main/cpp/CMakeLists.txt Show resolved Hide resolved

android-fast-dump/src/main/cpp/CMakeLists.txt Show resolved Hide resolved

android-fast-dump/src/main/cpp/CMakeLists.txt Show resolved Hide resolved

alhah added 3 commits May 13, 2021 20:36

Resolve some comments for native code

5cb1757

Signed-off-by: xueqiushi <xueqiushi@kuaishou.com>

Add fallback for fast dump failed, maybe child process killed.

ea6ceaf

Signed-off-by: xueqiushi <xueqiushi@kuaishou.com>

Update API changes

c41e187

Signed-off-by: xueqiushi <xueqiushi@kuaishou.com>

Minor refactorings / cleanups / doc

e6e02bb

pyricau force-pushed the alhah/fast-dump branch from d360513 to e6e02bb Compare September 16, 2021 22:58

alhah force-pushed the alhah/fast-dump branch from 2bc9459 to f2c9fe3 Compare September 20, 2021 08:33

Merge branch 'main' into alhah/fast-dump

31b1d55

pyricau mentioned this pull request Dec 23, 2021

New EventListener + HeapDumper APIs #2237

Merged

pyricau closed this Apr 2, 2022

alhah mentioned this pull request Sep 8, 2022

Add a fast dumper with HeapDumper API. #2387

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement fast dump #2047 #2121

Implement fast dump #2047 #2121

alhah commented May 5, 2021

CLAassistant commented May 5, 2021 •

edited

Loading

pyricau commented May 11, 2021

pyricau May 11, 2021

pyricau May 11, 2021

alhah May 12, 2021

pyricau May 11, 2021

alhah May 12, 2021

pyricau May 12, 2021

alhah May 12, 2021

pyricau May 12, 2021

pyricau May 11, 2021

alhah May 12, 2021

alhah May 12, 2021

pyricau May 11, 2021

sstallion left a comment

alhah commented Jun 29, 2021

pyricau commented Jun 29, 2021

alhah commented Jun 29, 2021

pyricau commented Sep 16, 2021

alhah commented Sep 20, 2021

alhah commented Dec 22, 2021

pyricau commented Dec 22, 2021 •

edited

Loading

pyricau commented Jan 10, 2022

alhah commented Jan 11, 2022

pyricau commented Apr 2, 2022

alhah commented Apr 6, 2022

alhah commented May 3, 2022

pyricau commented May 3, 2022

alhah commented May 4, 2022

alhah commented May 23, 2022 •

edited

Loading

Implement fast dump #2047 #2121

Implement fast dump #2047 #2121

Conversation

alhah commented May 5, 2021

CLAassistant commented May 5, 2021 • edited Loading

pyricau commented May 11, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sstallion left a comment

Choose a reason for hiding this comment

alhah commented Jun 29, 2021

pyricau commented Jun 29, 2021

alhah commented Jun 29, 2021

pyricau commented Sep 16, 2021

alhah commented Sep 20, 2021

alhah commented Dec 22, 2021

pyricau commented Dec 22, 2021 • edited Loading

pyricau commented Jan 10, 2022

alhah commented Jan 11, 2022

pyricau commented Apr 2, 2022

alhah commented Apr 6, 2022

alhah commented May 3, 2022

pyricau commented May 3, 2022

alhah commented May 4, 2022

alhah commented May 23, 2022 • edited Loading

CLAassistant commented May 5, 2021 •

edited

Loading

pyricau commented Dec 22, 2021 •

edited

Loading

alhah commented May 23, 2022 •

edited

Loading