Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2275253002:
Add a workaround to work with dalvik running on x86 (Closed)
|
Created:
4 years, 3 months ago by Takashi Toyoshima Modified:
4 years, 3 months ago CC:
catapult-reviews_chromium.org Base URL:
https://chromium.googlesource.com/external/github.com/catapult-project/catapult.git@master Target Ref:
refs/heads/master Project:
catapult Visibility:
Public. |
DescriptionAdd a workaround to work with dalvik running on x86
On KitKat, libdvm.so shows linker warning unless ART is enabled.
This is a bug of dalvik interpreter writtein in assembly.
We can safely ignore this warning message.
Committed: https://chromium.googlesource.com/external/github.com/catapult-project/catapult/+/be813143dbaed76aa8057732df9e3bb81ab12689
Patch Set 1 #
Total comments: 4
Patch Set 2 : add version and abi checks #
Total comments: 2
Patch Set 3 : allow KITKAT_WATCH too #Messages
Total messages: 17 (7 generated)
Description was changed from ========== Add a workaround to work with dalvik running on x86 On KitKat, libdvm.so shows linker warning unless ART is enabled. This is a bug of dalvik interpreter writtein in assembly. We can safely ignore this warning message. BUG=catapult:# ========== to ========== Add a workaround to work with dalvik running on x86 On KitKat, libdvm.so shows linker warning unless ART is enabled. This is a bug of dalvik interpreter writtein in assembly. We can safely ignore this warning message. BUG=catapult:# ==========
toyoshim@chromium.org changed reviewers: + nduca@chromium.org
On reproducing KK specific test failure, I notice this problem. Can you take a look?
sullivan@chromium.org changed reviewers: + jbudorick@chromium.org, sullivan@chromium.org - nduca@chromium.org
Switching reviewer to jbudorick since nduca is OOO and not very familiar w/devil.
https://codereview.chromium.org/2275253002/diff/1/devil/devil/android/device_... File devil/devil/android/device_utils.py (right): https://codereview.chromium.org/2275253002/diff/1/devil/devil/android/device_... devil/devil/android/device_utils.py:492: # Old Android running on x86 may show following messages. If this only applies to dalvik+x86, it should be gated on those.
https://codereview.chromium.org/2275253002/diff/1/devil/devil/android/device_... File devil/devil/android/device_utils.py (right): https://codereview.chromium.org/2275253002/diff/1/devil/devil/android/device_... devil/devil/android/device_utils.py:492: # Old Android running on x86 may show following messages. Yeah, this is a x86 assembly bug in libdvm.so, DSO for Dalvik VM. I happen to know this because I fixed it when I worked in ARC team. So, does "should be gated" mean following workaround should be used only for Dalvik/x86? I'm not familiar with catapult code base, but is it easy to check CPU and runtime variant of the target device here? 'getprop {ro.build.id|ro.product.cpu.abi|persist.sys.dalvik.vm.lib}' would help to have such check via adb shell, but it looks overdone for this check to me.
https://codereview.chromium.org/2275253002/diff/1/devil/devil/android/device_... File devil/devil/android/device_utils.py (right): https://codereview.chromium.org/2275253002/diff/1/devil/devil/android/device_... devil/devil/android/device_utils.py:492: # Old Android running on x86 may show following messages. On 2016/08/26 11:01:36, toyoshim wrote: > Yeah, this is a x86 assembly bug in libdvm.so, DSO for Dalvik VM. I happen to > know this because I fixed it when I worked in ARC team. > > So, does "should be gated" mean following workaround should be used only for > Dalvik/x86? Yes. > I'm not familiar with catapult code base, but is it easy to check CPU and > runtime variant of the target device here? DeviceUtils.build_version_sdk will let you check that the device is on K or below, while DeviceUtils.product_cpu_abi should let you check for x86. Neither should incur an additional adb call. > > 'getprop {ro.build.id|ro.product.cpu.abi|persist.sys.dalvik.vm.lib}' would help > to have such check via adb shell, but it looks overdone for this check to me.
Description was changed from ========== Add a workaround to work with dalvik running on x86 On KitKat, libdvm.so shows linker warning unless ART is enabled. This is a bug of dalvik interpreter writtein in assembly. We can safely ignore this warning message. BUG=catapult:# ========== to ========== Add a workaround to work with dalvik running on x86 On KitKat, libdvm.so shows linker warning unless ART is enabled. This is a bug of dalvik interpreter writtein in assembly. We can safely ignore this warning message. ==========
https://codereview.chromium.org/2275253002/diff/1/devil/devil/android/device_... File devil/devil/android/device_utils.py (right): https://codereview.chromium.org/2275253002/diff/1/devil/devil/android/device_... devil/devil/android/device_utils.py:492: # Old Android running on x86 may show following messages. Thanks. That's exactly what I need here, and sorry I should find it before asking you by a simple search.
lgtm w/ nit https://codereview.chromium.org/2275253002/diff/20001/devil/devil/android/dev... File devil/devil/android/device_utils.py (right): https://codereview.chromium.org/2275253002/diff/20001/devil/devil/android/dev... devil/devil/android/device_utils.py:494: and self.build_version_sdk == version_codes.KITKAT nit: Should this be <= (and maybe <= KITKAT_WATCH)?
https://codereview.chromium.org/2275253002/diff/20001/devil/devil/android/dev... File devil/devil/android/device_utils.py (right): https://codereview.chromium.org/2275253002/diff/20001/devil/devil/android/dev... devil/devil/android/device_utils.py:494: and self.build_version_sdk == version_codes.KITKAT Good point. I checked the KitKat Wear image for x86 just now, and confirmed that it actually showed the same message. The next patch set fixes this check to contain the wear.
The CQ bit was checked by toyoshim@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from jbudorick@chromium.org Link to the patchset: https://codereview.chromium.org/2275253002/#ps40001 (title: "allow KITKAT_WATCH too")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Add a workaround to work with dalvik running on x86 On KitKat, libdvm.so shows linker warning unless ART is enabled. This is a bug of dalvik interpreter writtein in assembly. We can safely ignore this warning message. ========== to ========== Add a workaround to work with dalvik running on x86 On KitKat, libdvm.so shows linker warning unless ART is enabled. This is a bug of dalvik interpreter writtein in assembly. We can safely ignore this warning message. Committed: https://chromium.googlesource.com/external/github.com/catapult-project/catapu... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/external/github.com/catapult-project/catapu... |
