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

[Microsoft.Android.Runtime.NativeAOT] Parse debug.mono.log #9824

Merged
merged 2 commits into from
Feb 24, 2025

Conversation

jonpryor
Copy link
Member

Context: https://github.com/dotnet/java-interop/blob/9dea87dc1f3052ed0f9499c9858b27c83e7d139e/samples/Hello-NativeAOTFromAndroid/README.md

The NativeAOT sample and related Microsoft.Android.Runtime.NativeAOT assembly is based in part on the Hello-NativeAOTFromAndroid sample in dotnet/java-interop, which contains this comment about Logging:

The (very!) extensive logging around JNI Global and Local
references mean that this sample should not be used as-is for
startup timing comparison.

…because adb logcat is absolutely spammed by LREF and GREF logs.

The "solution" in .NET for Android "proper" is to use the debug.mono.log system property to control logging. By default, LREF and GREF logs are disabled, but can be enabled by setting the debug.mono.log system property property:

adb shell setprop debug.mono.log gref       # only grefs
adb shell setprop debug.mono.log lref,gref  # lrefs & grefs
adb shell setprop debug.mono.log all        # EVERYTHING

Begin plumbing support for debug.mono.log into JavaInteropRuntime. This allows us to e.g. have LREF and GREF logging disabled by default, allowing for (somewhat) more representative startup timing comparisons, while allowing these logs to be enabled when needed.

Context: https://github.com/dotnet/java-interop/blob/9dea87dc1f3052ed0f9499c9858b27c83e7d139e/samples/Hello-NativeAOTFromAndroid/README.md

The NativeAOT sample and related `Microsoft.Android.Runtime.NativeAOT`
assembly is based in part on the `Hello-NativeAOTFromAndroid` sample
in dotnet/java-interop, which contains this comment about Logging:

> The (very!) extensive logging around JNI Global and Local
> references mean that this sample should not be used as-is for
> startup timing comparison.

…because `adb logcat` is absolutely *spammed* by LREF and GREF logs.

The "solution" in .NET for Android "proper" is to use the
`debug.mono.log` system property to control logging.  By default,
LREF and GREF logs are disabled, but can be enabled by setting
the `debug.mono.log` system property property:

	adb shell setprop debug.mono.log gref       # only grefs
	adb shell setprop debug.mono.log lref,gref  # lrefs & grefs
	adb shell setprop debug.mono.log all        # EVERYTHING

Begin plumbing support for `debug.mono.log` into `JavaInteropRuntime`.
This allows us to e.g. have LREF and GREF logging *disabled by default*,
allowing for (somewhat) more representative startup timing comparisons,
while allowing these logs to be enabled when needed.

Choose a reason for hiding this comment

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

Copilot reviewed 1 out of 2 changed files in this pull request and generated no comments.

Files not reviewed (1)
  • src/Microsoft.Android.Runtime.NativeAOT/Microsoft.Android.Runtime.NativeAOT.csproj: Language not supported
Change the system property checked from `debug.mono.log`
to `debug.dotnet.log`.

One of the features of `debug.mono.log` is that lref & gref logs
can go to specific files:

    adb shell setprop debug.mono.log lref=…/lref.txt,gref=…/gref.txt

Cleanup and retrofit so that this is also supported: introduce a new
`DiagnosticSettings` type to handle system property parsing and
underlying state, along with (possibly common) paths:

	adb shell mkdir -p /sdcard/Documents/jonp
	adb shell chmod 777 /sdcard/Documents/jonp
	adb shell setprop debug.dotnet.log gref=/sdcard/Documents/jonp/jni.txt,lref=/sdcard/Documents/jonp/jni.txt

will send lref & gref logs to `/sdcard/Documents/jonp/jni.txt`.
Or, they can be separate files.

However, the file can only be created once in this circumstance;
if the file already exists, we'll write a log message and then
dump stuff to `adb logcat`:

	E NativeAot: Failed to open log file `/sdcard/Documents/jonp/jni.txt`: System.UnauthorizedAccessException: UnauthorizedAccess_IODenied_Path, /sdcard/Documents/jonp/jni.txt
	E NativeAot:  ---> System.IO.IOException: Permission denied
	…
	D NativeAot:LREF: +l+ lrefc 1 handle 0x723ff2501d/L from thread ''(1)
	…

The introduction of `DiagnosticSettings` allows `debug.dotnet.log`/etc.
to contain additional data, beyond what a `[Flags]` enum could hold.
Comment on lines +20 to +52
public TextWriter? GrefLog {
get {
if (!LogJniGlobalReferences) {
return null;
}
return ((LrefPath != null && LrefPath == GrefPath)
? GrefLrefLog ??= CreateWriter (LrefPath)
: null)
??
((GrefPath != null)
? CreateWriter (GrefPath)
: null)
??
new LogcatTextWriter (AndroidLogLevel.Debug, "NativeAot:GREF");
}
}

public TextWriter? LrefLog {
get {
if (!LogJniLocalReferences) {
return null;
}
return ((LrefPath != null && LrefPath == GrefPath)
? GrefLrefLog ??= CreateWriter (LrefPath)
: null)
??
((LrefPath != null)
? CreateWriter (LrefPath)
: null)
??
new LogcatTextWriter (AndroidLogLevel.Debug, "NativeAot:LREF");
}
}
Copy link
Member

Choose a reason for hiding this comment

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

I read through these twice, as it seemed easy to have a typo. I think they are right.

@jonpryor jonpryor merged commit b217dca into main Feb 24, 2025
58 checks passed
@jonpryor jonpryor deleted the dev/jonp/jonp-nativeaot-debug.mono.log-parsing branch February 24, 2025 18:05
jonathanpeppers added a commit that referenced this pull request Feb 24, 2025
Context: #9824

With `debug.dotnetlog` support implemented, the lref/gref logging
seems sufficient for debugging Java interop (or GC)-related issues.

Remove other logging in `NativeAotTypeManager` that seems unnecessary
and will yield better startup performance.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants