New issue
Advanced search Search tips

Issue 801260 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

JNI function calls are not tracked by task annotator

Project Member Reported by ssid@chromium.org, Jan 11 2018

Issue description

The native execution which are triggered by jni calls sometimes do not get register in task annotator. This causes lot of unknown memory usage in heap profiler.
We need a way to trace native allocations.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jan 22 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/affe36d3b5bcb2e4595f7c976b65d9332b17518d

commit affe36d3b5bcb2e4595f7c976b65d9332b17518d
Author: Siddhartha <ssid@chromium.org>
Date: Mon Jan 22 22:48:16 2018

Add heap profiler events for jni generated functions

The native functions invoked are sometimes not run as a task which is
registered with TaskAnnotator. Add events on generated jni functions to
be able to trace them.
Causes 80KB regression on the binary size, so use for temporary
debugging in Dev channel and revert CL.

BUG=801260

Change-Id: I80931fc07d74ed27edebe3cf35bce689f8138099
Reviewed-on: https://chromium-review.googlesource.com/853081
Commit-Queue: Siddhartha S <ssid@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531028}
[modify] https://crrev.com/affe36d3b5bcb2e4595f7c976b65d9332b17518d/base/android/jni_generator/SampleForTests_jni.golden
[modify] https://crrev.com/affe36d3b5bcb2e4595f7c976b65d9332b17518d/base/android/jni_generator/jni_generator.py
[modify] https://crrev.com/affe36d3b5bcb2e4595f7c976b65d9332b17518d/base/android/jni_generator/jni_generator_helper.h
[modify] https://crrev.com/affe36d3b5bcb2e4595f7c976b65d9332b17518d/base/android/jni_generator/testInnerClassNatives.golden
[modify] https://crrev.com/affe36d3b5bcb2e4595f7c976b65d9332b17518d/base/android/jni_generator/testInnerClassNativesBothInnerAndOuter.golden
[modify] https://crrev.com/affe36d3b5bcb2e4595f7c976b65d9332b17518d/base/android/jni_generator/testInnerClassNativesMultiple.golden
[modify] https://crrev.com/affe36d3b5bcb2e4595f7c976b65d9332b17518d/base/android/jni_generator/testMultipleJNIAdditionalImport.golden
[modify] https://crrev.com/affe36d3b5bcb2e4595f7c976b65d9332b17518d/base/android/jni_generator/testNativeExportsOnlyOption.golden
[modify] https://crrev.com/affe36d3b5bcb2e4595f7c976b65d9332b17518d/base/android/jni_generator/testNatives.golden
[modify] https://crrev.com/affe36d3b5bcb2e4595f7c976b65d9332b17518d/base/android/jni_generator/testNativesLong.golden
[modify] https://crrev.com/affe36d3b5bcb2e4595f7c976b65d9332b17518d/base/android/jni_generator/testSingleJNIAdditionalImport.golden

Cc: mbonadei@chromium.org
Project Member

Comment 3 by bugdroid1@chromium.org, Jan 23 2018

The following revision refers to this bug:
  https://webrtc.googlesource.com/src.git/+/63e83c77ae81730a78ec4d5bf0465f25970f867a

commit 63e83c77ae81730a78ec4d5bf0465f25970f867a
Author: Mirko Bonadei <mbonadei@webrtc.org>
Date: Tue Jan 23 10:33:26 2018

Forward fix jni_generator_helper.h.

In crrev.com/531028, the JNI generator starts to add heap profiler
events to JNI generated functions.

This will cause a ~80KiB regression and at the moment it is breaking
the Chromium Roll into WebRTC.

This CL defines a void macro to re-enable the Chromium Roll avoiding
the size regression.

Bug: chromium:801260
Change-Id: I9543299199c4e14b6b9b235c5cb98c0d53cf29ea
Reviewed-on: https://webrtc-review.googlesource.com/43021
Reviewed-by: Sami Kalliomäki <sakal@webrtc.org>
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#21730}
[modify] https://crrev.com/63e83c77ae81730a78ec4d5bf0465f25970f867a/sdk/android/src/jni/jni_generator_helper.h

I am following this to remove the WebRTC workaround when crrev.com/531028 will be reverted.

Is it possible to restrict the scope of this kind of changes to chrome? I guess it is really complicated and this is the easiest way to get the job done. But if there is a way it would be great to enable this feature on a per-project basis.
Project Member

Comment 5 by bugdroid1@chromium.org, Feb 27 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ef514ed61bd93c7446da3842f035b418e7ab41af

commit ef514ed61bd93c7446da3842f035b418e7ab41af
Author: Siddhartha S <ssid@chromium.org>
Date: Tue Feb 27 00:17:15 2018

Revert "Add heap profiler events for jni generated functions"

This reverts commit affe36d3b5bcb2e4595f7c976b65d9332b17518d.

Reason for revert: Experimental CL. Reverting before branch cut.

Original change's description:
> Add heap profiler events for jni generated functions
>
> The native functions invoked are sometimes not run as a task which is
> registered with TaskAnnotator. Add events on generated jni functions to
> be able to trace them.
> Causes 80KB regression on the binary size, so use for temporary
> debugging in Dev channel and revert CL.
>
> BUG=801260
>
> Change-Id: I80931fc07d74ed27edebe3cf35bce689f8138099
> Reviewed-on: https://chromium-review.googlesource.com/853081
> Commit-Queue: Siddhartha S <ssid@chromium.org>
> Reviewed-by: agrieve <agrieve@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#531028}

TBR=ssid@chromium.org,agrieve@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 801260
Change-Id: I5fb690cb4fa24ee4f2305d2b90264cae40f3cefd
Reviewed-on: https://chromium-review.googlesource.com/938561
Reviewed-by: agrieve <agrieve@chromium.org>
Commit-Queue: Siddhartha S <ssid@chromium.org>
Cr-Commit-Position: refs/heads/master@{#539329}
[modify] https://crrev.com/ef514ed61bd93c7446da3842f035b418e7ab41af/base/android/jni_generator/SampleForTests_jni.golden
[modify] https://crrev.com/ef514ed61bd93c7446da3842f035b418e7ab41af/base/android/jni_generator/jni_generator.py
[modify] https://crrev.com/ef514ed61bd93c7446da3842f035b418e7ab41af/base/android/jni_generator/jni_generator_helper.h
[modify] https://crrev.com/ef514ed61bd93c7446da3842f035b418e7ab41af/base/android/jni_generator/testInnerClassNatives.golden
[modify] https://crrev.com/ef514ed61bd93c7446da3842f035b418e7ab41af/base/android/jni_generator/testInnerClassNativesBothInnerAndOuter.golden
[modify] https://crrev.com/ef514ed61bd93c7446da3842f035b418e7ab41af/base/android/jni_generator/testInnerClassNativesMultiple.golden
[modify] https://crrev.com/ef514ed61bd93c7446da3842f035b418e7ab41af/base/android/jni_generator/testMultipleJNIAdditionalImport.golden
[modify] https://crrev.com/ef514ed61bd93c7446da3842f035b418e7ab41af/base/android/jni_generator/testNativeExportsOnlyOption.golden
[modify] https://crrev.com/ef514ed61bd93c7446da3842f035b418e7ab41af/base/android/jni_generator/testNatives.golden
[modify] https://crrev.com/ef514ed61bd93c7446da3842f035b418e7ab41af/base/android/jni_generator/testNativesLong.golden
[modify] https://crrev.com/ef514ed61bd93c7446da3842f035b418e7ab41af/base/android/jni_generator/testSingleJNIAdditionalImport.golden

Project Member

Comment 6 by bugdroid1@chromium.org, Feb 27 2018

The following revision refers to this bug:
  https://webrtc.googlesource.com/src.git/+/f7706aecdc1f5b0b138a313fa17c80c84bf26ea0

commit f7706aecdc1f5b0b138a313fa17c80c84bf26ea0
Author: siddhartha sivakumar <ssid@chromium.org>
Date: Tue Feb 27 15:10:39 2018

Revert "Forward fix jni_generator_helper.h."

This reverts commit 63e83c77ae81730a78ec4d5bf0465f25970f867a.

Reason for revert: JNI generator is not using the heap profiler
anymore.

Original change's description:
> Forward fix jni_generator_helper.h.
>
> In crrev.com/531028, the JNI generator starts to add heap profiler
> events to JNI generated functions.
>
> This will cause a ~80KiB regression and at the moment it is breaking
> the Chromium Roll into WebRTC.
>
> This CL defines a void macro to re-enable the Chromium Roll avoiding
> the size regression.
>
> Bug: chromium:801260
> Change-Id: I9543299199c4e14b6b9b235c5cb98c0d53cf29ea
> Reviewed-on: https://webrtc-review.googlesource.com/43021
> Reviewed-by: Sami Kalliomäki <sakal@webrtc.org>
> Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#21730}

TBR=mbonadei@webrtc.org,magjed@webrtc.org,sakal@webrtc.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: chromium:801260
Change-Id: I7dac211b89d8206dc461af0a17b6d53cc8661b2a
Reviewed-on: https://webrtc-review.googlesource.com/58040
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Magnus Jedvert <magjed@webrtc.org>
Reviewed-by: Sami Kalliomäki <sakal@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#22206}
[modify] https://crrev.com/f7706aecdc1f5b0b138a313fa17c80c84bf26ea0/sdk/android/src/jni/jni_generator_helper.h

Status: Assigned (was: Untriaged)
This bug has an owner, thus, it's been triaged. Changing status to "assigned".

Sign in to add a comment