Name spawned threads - Githubissues

rust-mobile / android-activity

Glue for building Rust applications on Android with NativeActivity or GameActivity

238 stars 46 forks source link

Name spawned threads #145

Closed fornwall closed 9 months ago

fornwall commented 10 months ago

Name spawned threads to make things more clear during debugging and profiling.

Max thread name is 15, so we can't fit more descriptive names like android-activity-main (but open for some other naming scheme).

MarijnS95 commented 10 months ago

This probably conflicts with https://github.com/rust-mobile/android-activity/pull/133, which IMO should be merged first to stay chronological.

rib commented 9 months ago

Nice.

Since we know we're running on Android maybe we can drop the 'android-' prefix even, unless the Java main thread is already called 'main' perhaps?

fornwall commented 9 months ago

Since we know we're running on Android maybe we can drop the 'android-' prefix even, unless the Java main thread is already called 'main' perhaps?

The reasoning for the android- prefix was to improve clarity for someone listing thread names for an application, just having main and stdio is a bit more nondescriptive/generic.

But how about:

native-activity and game-activity for the main threads - in that way one call also clearly tell if the game or native activity is used.
stdio-to-logcat as the stdio-forwarder thread - shows what it is doing. ?

rib commented 9 months ago

Yeah, stdio-to-logcat sounds a bit more descriptive of that thread's function.

I think main would be a pretty typical name for an application's main thread (and what I'd also find intuitive if I was looking at a list of threads in a profiler) but my main :) concern with that on Android is that that it's common to refer to the application's Java thread as the 'main' thread and maybe that's even what it's named at the system level?

I wouldn't be super keen on having 'activity' in the name since the Activity is really something that exists on the JVM side of things which might then be misleading.

Something like android_main might be appropriate if we need to disambiguate from a Java 'main' thread where this is the name of the actual fn android_main() native function that's called.

fornwall commented 9 months ago

I wouldn't be super keen on having 'activity' in the name since the Activity is really something that exists on the JVM side of things which might then be misleading.

Agreed!

Something like android_main might be appropriate if we need to disambiguate from a Java 'main' thread where this is the name of the actual fn android_main() native function that's called.

Sounds good - let's use android_main?

Pushed a commit to use android_main and stdio-to-logcat as thread names now.

rib commented 9 months ago

Okey, @MarijnS95's updated PR for #133 ended up naming the stdio-to-logcat thread as proposed here (it made sense to do the rename while anyway factoring out a shared utility for spawning that thread), so its just the android_main thread left to name here. Yeah I think 'android_main' seems reasonable to me based on the name of the top-level native function for that thread.

fornwall commented 9 months ago

Okey, @MarijnS95's updated PR for https://github.com/rust-mobile/android-activity/pull/133 ended up naming the stdio-to-logcat thread as proposed here (it made sense to do the rename while anyway factoring out a shared utility for spawning that thread), so its just the android_main thread left to name here

👍 - merge conflict has now been resolved.