Where should "class verification" docs live?
14 views
Skip to first unread message
Nate Fischer
Jun 20, 2019, 4:23:18 PM6/20/19
to java, Andrew Grieve, Changwan Ryu
Hi all,
I wrote up some markdown to explain what is ART class verification, how to investigate class verification problems, and how to fix those problems in chromium. The CL is here, and you can look at the rendered markdown for context.
One question raised on the CL, which I'd like this list's input regarding, is where should this document live?
The currently proposed home is in //build/android/docs/, which has the benefit of proximity to the list_class_verification_failures.py script. But Andrew pointed out the "Chromium's solution" section reads more like a best practice/styleguide (which would suggest moving the doc next to the Java style guide). Yet another idea would be to leave the doc where it is, but add a section in the styleguide to point to the build/android/docs/ page. Does anyone have preferences?
Nate Fischer
Jun 25, 2019, 12:44:49 PM6/25/19
to java, Andrew Grieve, Changwan Ryu
For the time being, I've landed the CL as-is in the //build/android/docs/ folder. For those interested, you can read the rendered markdown here.
If folks still want to chime in, I'm more than happy to move the docs should we come to a consensus. For the time being, I have not modified the Java style guide, but may propose a CL if others think it's worth recommending.
Peter Wen
Jun 26, 2019, 8:37:20 AM6/26/19
to Nate Fischer, java, Andrew Grieve, Changwan Ryu
This is really helpful, Nate! Thank you for documenting the history and reasoning behind the way we API methods are wrapped.
I for one think that this should have a link from the Chromium java style guide.
Peter
--
You received this message because you are subscribed to the Google Groups "java" group.
To unsubscribe from this group and stop receiving emails from it, send an email to java+uns...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/java/CACiwn8bxmsrkKPvUoSTjeqAaJi39d6ucSdehFpd65sqqHM2yeA%40mail.gmail.com.
Tommy Nyquist
Jun 27, 2019, 5:08:02 PM6/27/19
to Peter Wen, Nate Fischer, java, Andrew Grieve, Changwan Ryu
This was a great read! Thank you so much, Nate!
I don't think we've done this type of outlining consistently in our code base, but I think it might make sense to provide guidance on this. I agree with Peter that we should find a relevant place to shorten this into "this is how you do it" part in the style guide and then link out to the doc you wrote for details. Any chance you could pick this up, Nate?
I was also wondering; any chance we could get metrics on this from the wild (UMA?) to see how much impact this has in practice? Or do we not have visibility into this in prod builds of the system image?
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/java/CAKNWQC6wvd86dkrCTBqnMyaatb%2BjDsB2a5p6WJKAeCgNWn5aXA%40mail.gmail.com.
Nate Fischer
Jul 10, 2019, 10:38:46 PM7/10/19
to Tommy Nyquist, Peter Wen, java, Andrew Grieve, Changwan Ryu
We don't have UMA to measure total verification time, nor do I know if it's feasible to measure this within Chrome itself. We do have a histogram measuring WebView startup, but I don't remember if it captured the impact of the initial out-of-lining we landed, and WebView's UMA data from that period may no longer be accessible for unrelated reasons.
I've filed crbug/982999 to update the style guide.
Peter Wen
Jul 11, 2019, 9:04:57 AM7/11/19
to Nate Fischer, Tommy Nyquist, java, Andrew Grieve, Changwan Ryu
Hi Nate,
I get this error when I use the script (after installing monochrome_apk). Am I missing anything?
$ build/android/list_class_verification_failures.py --package="org.chromium.chrome"
W 0.000s Main Skipping deobfuscation because no map file was provided.
Traceback (most recent call last):
File "build/android/list_class_verification_failures.py", line 283, in <module>
main()
File "build/android/list_class_verification_failures.py", line 279, in main
args.hide_summary, workdir)
File "build/android/list_class_verification_failures.py", line 212, in RealMain
_AdbOatDumpForPackage(device, package, file_on_device.name)
File "build/android/list_class_verification_failures.py", line 123, in _AdbOatDumpForPackage
odex_file = PathToDexForPlatformVersion(device, package_name)
File "build/android/list_class_verification_failures.py", line 117, in PathToDexForPlatformVersion
raise DeviceOSError('Unable to find odex file ' + odex_file)
__main__.DeviceOSError: Unable to find odex file /data/app/org.chromium.chrome-jWHkJPWJUFsI2oAel-PTwQ==/oat/arm64/base.odex
Thanks,
Peter
Nate Fischer
Jul 11, 2019, 3:28:00 PM7/11/19
to Peter Wen, Tommy Nyquist, java, Andrew Grieve, Changwan Ryu
Do you mean monochrome_public_apk? monochrome_apk (downstream) has a different package name.
If that's the cause, then I could probably create better error messages in the script. If you actually did install org.chromium.chrome, then this is probably a script bug (you can file a crbug against me and include --verbose logging).
Peter Wen
Jul 11, 2019, 4:37:15 PM7/11/19
to Nate Fischer, Tommy Nyquist, java, Andrew Grieve, Changwan Ryu
Ah, I did monochrome_apk instead. Will try with monochrome_public_apk later, thanks Nate!
Peter
Nate Fischer
Jul 11, 2019, 4:45:00 PM7/11/19
to Peter Wen, Tommy Nyquist, java, Andrew Grieve, Changwan Ryu
monochrome_apk should work too, you just need to use the correct package name (com.google.android.apps.chrome, unless you've set the android_channel GN arg). I've filed crbug/983280 to improve the error output for this case.
Reply all
Reply to author
Forward
0 new messages