OS logging integrated with `trace` for iOS #1140

elijah-semyonov · 2024-02-26T13:29:56Z

Proposed Changes

Integrate iOS logging with traces API in ui:util modules.
Additional traces in ui in skikoMain source set.

Testing

Test: N/A

API Change

API added to iOS source set

// androidx.compose.ui.util
@ExperimentalComposeUiApi
fun enableTraceOSLog()

…into es/ios-traces

MatkovIvan · 2024-02-26T15:11:40Z

compose/runtime/runtime/src/linuxArm64Main/kotlin/androidx/compose/runtime/Trace.linuxArm64.kt

Why do you need multiple copies for each linux/win arch? It should be just one like nonIosNativeMain or just linuxMain. Additional gradle setup is required of course.

Will fix it

MatkovIvan · 2024-02-26T15:13:26Z

compose/runtime/runtime/src/uikitMain/kotlin/androidx/compose/runtime/Trace.uikit.kt

+@ExperimentalComposeApi
+fun enableTraceOSLog() {
+    if (traceImpl == null) {
+        traceImpl = CMPOSLogger(categoryName = "androidx.compose.runtime")


It seems it should work on all Apple platforms - macOS, tvOS, watchOS

No, this class is implemented in UIKitUtils framework, which is built for iOS now.

MatkovIvan · 2024-02-26T15:15:26Z

compose/ui/ui-util/src/wasmJsMain/kotlin/androidx/compose/ui/util/Trace.wasmJs.kt

The same - you should combine js and wasmJs into one file/sourceset - webMain

MatkovIvan · 2024-02-26T15:16:37Z

compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/scene/BaseComposeScene.skiko.kt

+        return trace("BaseComposeScene:render") {
+            postponeInvalidation {


Let's add trace inside postponeInvalidation and pass tag as parameter

Added trace, but didn't get what you mean under pass tag as a parameter, can you elaborate?

MatkovIvan · 2024-02-26T15:17:49Z

compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/scene/SingleLayerComposeScene.skiko.kt

@@ -171,7 +172,7 @@ private class SingleLayerComposeSceneImpl(
        mainOwner.measureAndLayout()
    }

-    override fun draw(canvas: Canvas) {
+    override fun draw(canvas: Canvas) = trace("SingleLayerComposeSceneImpl:draw") {


Move it to owner, no need to log "SingleLayerComposeSceneImpl"

MatkovIvan · 2024-02-27T11:30:50Z

compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/scene/BaseComposeScene.skiko.kt

@@ -75,7 +75,7 @@ internal abstract class BaseComposeScene(
        private set

    private var isInvalidationDisabled = false
-    private inline fun <T> postponeInvalidation(crossinline block: () -> T): T {
+    private inline fun <T> postponeInvalidation(crossinline block: () -> T): T = trace("BaseComposeScene:postponeInvalidation") {


It's called from multiple places. Let's add label for trace in parameters of postponeInvalidation?

MatkovIvan · 2024-02-27T11:33:15Z

compose/ui/ui-uikit/src/uikitMain/objc/CMPUIKitUtils/CMPUIKitUtils/CMPOSLogger.m

+- (CMPOSLoggerInterval *)beginIntervalNamed:(NSString *)name {
+    CMPOSLoggerInterval *interval;
+
+    [_poolLock lock];


You're adding unconditional mutex lock in many critical places. It will probably impact performance

I doubt it, it's only around 20 calls per frame and critical section is very short and not congested due to almost all current calls begin on the same thread. Moreover, this logic is performed only if logging was enabled in the first place. It's not supposed to be used in production env.

MatkovIvan

Aside from unconditional adding mutexes, LGTM.

Should be reviewed by someone else too

igordmn

Other than "runtime" depending on "ui" - looks good to me

compose/runtime/runtime/build.gradle

compose/ui/ui-uikit/src/uikitMain/objc/CMPUIKitUtils/CMPUIKitUtils/CMPOSLoggerInterval.m

compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/scene/BaseComposeScene.skiko.kt

…aseComposeScene.skiko.kt Co-authored-by: Igor Demin <igordmn@users.noreply.github.com>

igordmn

Let's also remove it from the PR description

ASalavei and others added 23 commits February 21, 2024 13:15

Import ui:uikit sample

bd01722

Add logger stubs

9e31136

Merge remote-tracking branch 'origin/andrei.salavei/import-ui-uikit' …

5d7ba23

…into es/ios-traces

Add stubs for Trace.

41dbd87

Integrate CMPOSLogger with runtime.

a158f20

Add category name to logger

8f33b47

Add to Trace to ui.

0e3165a

Make trace impl private

0a6a45e

Change annotation

a2b8769

Add traces

e72cb33

Add traces

8171bb0

Modify trace format

d30099d

Fix bug

0571e97

Add todo

efdc56f

Add trace to skikoMain

42cf270

Move to ui-utils existing func

0fc7c34

Remove test class

bebcfc4

Remove Trace.jb.kt

8af1ac5

Remove unneeded files, import trace from other package

0a8999a

Add dependency on objc module to ui-utils

9695ea1

Add proper annotations

02f7be7

Fix compilation error

ba137e1

Add trace for wait signal

9a75279

elijah-semyonov requested review from MatkovIvan and igordmn February 26, 2024 13:29

elijah-semyonov self-assigned this Feb 26, 2024

elijah-semyonov added 2 commits February 26, 2024 14:47

Add extra traces

96ea8ee

Fix bin compat issue

806d73e

MatkovIvan requested changes Feb 26, 2024

View reviewed changes

Add trace

5fa15f6

elijah-semyonov added 5 commits February 27, 2024 09:25

Move trace

d9df692

Unify linux source sets in runtime

ee4adeb

Unify web targets

1cbaaa8

Remove files

b2951f5

Add space

7f279e9

elijah-semyonov requested a review from MatkovIvan February 27, 2024 09:17

Update license header

93cd6e7

MatkovIvan reviewed Feb 27, 2024

View reviewed changes

Add tags

5d703c9

MatkovIvan approved these changes Feb 27, 2024

View reviewed changes

elijah-semyonov marked this pull request as ready for review February 27, 2024 14:29

elijah-semyonov added the changes in API label Feb 27, 2024

igordmn requested changes Feb 28, 2024

View reviewed changes

elijah-semyonov and others added 3 commits February 28, 2024 12:35

Remove redundant _name variable

5f26c26

Update compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/scene/B…

af98cef

…aseComposeScene.skiko.kt Co-authored-by: Igor Demin <igordmn@users.noreply.github.com>

Minor refactor

c5735e4

elijah-semyonov requested a review from igordmn February 28, 2024 11:45

Remove dependency from runtime

b345cba

igordmn approved these changes Feb 28, 2024

View reviewed changes

elijah-semyonov merged commit 9e9f8be into jb-main Feb 29, 2024
6 checks passed

elijah-semyonov deleted the es/ios-traces branch February 29, 2024 15:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OS logging integrated with `trace` for iOS #1140

OS logging integrated with `trace` for iOS #1140

elijah-semyonov commented Feb 26, 2024 •

edited

MatkovIvan Feb 26, 2024

elijah-semyonov Feb 27, 2024

elijah-semyonov Feb 27, 2024

MatkovIvan Feb 26, 2024

elijah-semyonov Feb 27, 2024

MatkovIvan Feb 26, 2024

elijah-semyonov Feb 27, 2024 •

edited

MatkovIvan Feb 26, 2024

elijah-semyonov Feb 27, 2024

MatkovIvan Feb 26, 2024

elijah-semyonov Feb 27, 2024

MatkovIvan Feb 27, 2024

elijah-semyonov Feb 27, 2024

MatkovIvan Feb 27, 2024

elijah-semyonov Feb 27, 2024 •

edited

MatkovIvan left a comment •

edited

igordmn left a comment

igordmn left a comment

		return trace("BaseComposeScene:render") {
		postponeInvalidation {

OS logging integrated with trace for iOS #1140

OS logging integrated with trace for iOS #1140

Conversation

elijah-semyonov commented Feb 26, 2024 • edited

Proposed Changes

Testing

API Change

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

elijah-semyonov Feb 27, 2024 • edited

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

elijah-semyonov Feb 27, 2024 • edited

Choose a reason for hiding this comment

MatkovIvan left a comment • edited

Choose a reason for hiding this comment

igordmn left a comment

Choose a reason for hiding this comment

igordmn left a comment

Choose a reason for hiding this comment

OS logging integrated with `trace` for iOS #1140

OS logging integrated with `trace` for iOS #1140

elijah-semyonov commented Feb 26, 2024 •

edited

elijah-semyonov Feb 27, 2024 •

edited

elijah-semyonov Feb 27, 2024 •

edited

MatkovIvan left a comment •

edited