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

LeakCanary 3 #2511

Open
pyricau opened this issue May 17, 2023 · 1 comment
Open

LeakCanary 3 #2511

pyricau opened this issue May 17, 2023 · 1 comment
Milestone

Comments

@pyricau
Copy link
Member

pyricau commented May 17, 2023

What to expect.

Changes to configuration

  • No more global config objects, no more XML overrides.
  • LeakCanary breaks down into small components that can be assembled and configured ad hoc
  • LeakCanary will provide higher level and decoupled recipe / builder APIs that make it easy to assemble these components for the most common use cases.
  • We'll default back to dumping on the first leak, and provide the ability to configure that differently.

Startup

  • We'll provide a single artifact that depends on Jetpack Startup, is meant to be used in debuggable builds only, and sets up object watching + heap dump + heap analysis + plumber using standard recipes. Ideally this would not provide any access to LeakCanary APIs in the app, need to check if we can do this with Maven or not, basically bring the dependencies to runtime but don't expose the APIs.
  • Any additional configuration means removing that dependency and doing all the wiring manually using a recipe dependency, somewhere during startup.

Analysis now performed in LeakCanary app

  • The main LeakCanary artifact will not include any heap analysis code.
  • The standard recipe will dump the heap then send the heap dump to the LeakCanary app.
  • Once sent the heap dumps will not be stored in the source app.

App APIs

  • Sending will be through AIDL, or maybe by firing an intent (startService?). The work will likely be done through a WorkManager job with foreground notifications for faster CPU work.
  • This is nice because it means less risk of OOM in the source app. We can make our app a large heap app.
  • There'll be an API that takes an analysis already performed + an optional heap dump. This is useful when you need to customize the heap analysis or if you'd rather not send the heap dump to the LeakCanary app.
  • Alternatively heap dumps can be sent via share file intent, or pull through import from app.

New features

  • From a leak trace, click on any object and see its state + start navigating the heap graph.
  • Explore a heap dump treemap.
  • Maybe: upload traces to a backend?

Notifications

  • Once the heap dump is performed, everything happens in the LeakCanary app, which by default will emit notifications
  • That will be configurable once & for all per app through notification channels.

Security

  • The sender needs to be able to authenticate the LeakCanary app and verify its signature against a known signature.
  • We'll document a recipe for going through hprof stripping prior to sending the heap dump.
  • We'll document a recipe for doing the analysis in process and then not send the heap dump, only the analysis. That recipe will also be used for when you want to customize the analysis. We'll have to document how to notify the user of analysis progress, maybe.
  • We'll document how one could build their own version of the app, sign it and add that signature to the config of their sender app, as well as set up their own updates hosting.

Analytics

  • We'll probably want lightweight user analytics with the ability to disable them.
  • Similarly, having crash handling would be nice to know what's impacting people the most. If enabled we can show a "sorry" message explaining that the crash was sent so they don't need to file an issue.
  • If disabled then we can provide a way to share the crash.

Proguard / R8

  • We'll need to support optionally sending a R8 / proguard mapping file together with the heap dump.
  • Ideally we'd actually delete the gradle obfuscation plugin and find a volunteer to maintain it in a separate repo then point to that? It'd also have to ship a small artifact (or example code) for how to pass the proguard file.

Installing / upgrading the app

  • When the app isn't installed, we should display a dialog (activity?) or notification? Find some way to get the user to install the app.
  • We should host the app on the Play Store to benefit from auto upgrades.
  • We should also host the app on the web for side loading. Same signature so that we can verify it.
  • Not clear what to do with the heap dump if the app isn't installed yet. Some retry mechanism, or just drop it? Maybe don't dump the heap at all, check before that? "There are leaks in the app but you need the leakcanary app". And you can snooze?
  • We probably can't auto update side loaded (?) but we could check an URL and show a notification "a new version is available"
  • We could potentially add a gradle plugin that takes care of adding the debug dependency + installing (and upgrading?) the app. Extends the install tasks to additional check if the LeakCanary app is there, if it's up to date, and if not go and install / upgrade it. In that case, not sure if we can install the PlayStore app through adb (??) and whether that can play nicely (e.g. can you install v1 from play store then plugin updates to v2 from side loaded?)

App internals

Dependency updates

  • Kotlin updated to XX
  • Okio becomes an optional dependency as part of heap analysis code which is not included by default since the app now takes care of that.
  • TBD if the jetpack core dependencies for ObjectWatcher stay hidden or if we make them explicit. In any case, we'll bump them to avoid the issues with incompatible bytecode.

Removed old LeakCanary activity

  • We'll remove the embedded LeakCanary UI. Someone could bring it back as a 3rd party artifact, or build a more modern version of it, or leverage the code for the separate app. But we really shouldn't maintain 2 UIs here.
  • We can reevaluate this and maybe ship a LeakCanary UI Compose artifact that host parts of the leakcanary app embedded. But that might just be too much of a hassle.
  • This also means no more leak database, etc.
  • Disabling LeakCanary through UI won't be an option anymore. Devs can take care of this with custom configuration (recipe needed)
  • We'll remove the support for WorkManager (=> recipe), for LeakCanary out of process analysis (=> recipe).

Internals and APIs

  • Need to break down the LeakCanary internals a lot more to have reusable components that can be combined in different ways. One core goal is that the same code can be used interchangeably for UI tests, release checks, debug, and even JVM leak detection.
  • All the notification code prior to heap dump should be extracted out and replaceable
  • Replace the LeakCanary dumping heap toast with something that's not a toast (to avoid toast leaks), e.g. insert view on top of top most root vie that's a phone window.
  • Make it possible to swap out the LruCache impl, notably to use a thread safe impl in contexts where that matters. Or maybe make it thread safe by default.
  • Leverages coroutines when navigating heap dumps to support cancelation as we move around the UI.

Compose Desktop / Intellij Plugin

  • We can potentially reuse UI parts from the LeakCanary app to build a Compose Desktop app.
  • It still early but soon enough we'll be able to build IntelliJ plugins with Compose. So potentially we can make a LeakCanary IntelliJ plugin. This is a much more powerful alternative to the Android Studio heap dump explorer.
  • You could imagine removing the heap dump & analysis work from the device entirely, and the desktop app maintaining a socket connection to know when weak refs aren't cleared and trigger a dump + pull via adb then analyze on desktop.

New feature

  • Improved dominator computation if I can figure out a way to eliminate the very long tail of objects hanging from the forest root (i.e. look at what multiple dominators they have and see if merging or ignoring one is possible)
  • Ability to perform heap dump diffs, with useful outputs. Look at how BLeaks does it. The YourKit one seem useful but seem to ignore the tree shape, instead just counting number of instances.
@pyricau pyricau added this to the 3.0 milestone May 18, 2023
@pyricau
Copy link
Member Author

pyricau commented May 27, 2023

Conversation on coroutines / flow for LeakTracer with Bill:

package shark

import kotlin.coroutines.CoroutineContext
import kotlinx.coroutines.CoroutineDispatcher
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Deferred
import kotlinx.coroutines.Job
import kotlinx.coroutines.async
import kotlinx.coroutines.coroutineScope
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.FlowCollector
import kotlinx.coroutines.flow.SharedFlow
import shark.LeakTracer.TraceObjectsEvent.StartedBuildingLeakTraces

fun interface LeakTracer {

  // TODO What should we do about exceptions here? union error or exception?
  fun traceObjects(objectIds: Set<Long>): LeaksAndUnreachableObjects

  // main safety
  suspend fun traceObjectsSuspending(
    // coroutineContext: CoroutineDispatcher, // this doesn't allow me to say, "use my existing dispatcher!"
    coroutineContext: CoroutineContext, // constructor DI is probably a good approach for this
    // Be careful with context you call this on!
    //     * events callback should be called on the same CoroutineContext as traceObjectsSuspending
    // Also: make sure and honor exception contracts! (e.g. if `events` throws,
    // `traceObjectSuspending` should throw that exeption)
    //
    // Flows get the above invariants right, so if you build on top of them you should be good
    events: (TraceObjectsEvent)->Unit
  ): LeaksAndUnreachableObjects

  data class TraceObjectsAsyncProcess(
    val job: Job,
    val events: SharedFlow<TraceObjectsEvent>,
    val result: Deferred<LeaksAndUnreachableObjects>,
  )

  fun traceObjectsAsync(coroutineScope: CoroutineScope, objectIds: Set<Long>): TraceObjectsAsyncProcess

  suspend fun heapGraph(block: suspend HeapGraph.()->Unit)
  suspend fun blah () {

    coroutineScope {
      val result: Deferred<Int> = async {
        1
      }
    }
    val result = traceObjectsSuspending {
      // event handling here
    }
  }
  // Flow? AndroidX Biometrics API
  //

  sealed interface TraceObjectsEvent {
    object StartedBuildingLeakTraces :  TraceObjectsEvent
    object StartedInspectingObjects :  TraceObjectsEvent
    object StartedComputingNativeRetainedSize:  TraceObjectsEvent
    object StartedComputingJavaHeapRetainedSize:  TraceObjectsEvent
  }

  fun traceObjectsFlow(objectIds: Set<Long>) : Flow<TraceObjectsEvent>

  // what this ^^^^ is actually doing

  suspend fun myFunction(callback: suspend (TraceObjectsEvent)->Unit) {
    while (true) {
      callback(StartedBuildingLeakTraces)
    }

  }

  fun whatItIsDoing() = object : Flow<TraceObjectsEvent> {
    override suspend fun collect(collector: FlowCollector<TraceObjectsEvent>) {
      TODO("Not yet implemented")
    }
  }

  fun interface Factory {
    fun createFor(heapGraph: HeapGraph): LeakTracer
  }
}

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

No branches or pull requests

1 participant