Skip to content

Commit 19a6799

Browse files
mattsarettandi34
authored andcommitted
libjpeg should always use jmemnobs DO NOT MERGE
BUG:30259087 cherry-pick of Iae944ac8588769c3e24f25f64de847e58cc2ad2e (ag/678466) in master Conflicts: Android.mk original message: libjpeg should always use jmemnobs When using jmem-android with libjpeg, the client has the option to provide a value for maximum memory usage (or use a default value) during the decode. When this value for maximum memory usage would be exceeded, jmem-android backs data arrays with temp files to stay below the memory limit. The CL that defines this memory limit is at: https://skia.googlesource.com/skia/+/2295c6332512833760060d803cf6ad19a28adc51 However, it is unclear why the current limit was chosen. Unfortunately, these temp files are not guaranteed to have unique names, leading to bugs where multiple arrays are backed up to the same file. The original purpose of this CL was to fix this bug. However, attempting to fix this bug has led to a variety of additional questions. The creation of temp files is tricky in this type of library. Concerns about the creation of temp files include security, performance of writing to disk, and difficulty of finding the right directory to use from a library that could be used by an arbitrary process. Additionally, there have already been complaints about jmem-android from the users of the Java API. b/2791339 explains that writing to disk is extremely slow, and it causes users of the decoder to need WRITE_EXTERNAL_STORAGE permission in order to decode. This led to the creation of jmem-ashmem.c as the memory implementation for libjpeg to be used by apps. This acts in the same way as jmem-android up until the arbitrary maximum memory usage value is reached. At this point, jmem-ashmem uses ashmem to back up the data arrays instead of temp files. We first considered using jmem-ashmem all the time, since it was introduced as a faster alternative to jmem-android that does not require extra permissions and does not have known bugs. However, thinking further about jmem-ashmem, we feel that it is not a good alternative. It is quite odd to solve the problem of being "out of memory" (having reached the memory limit) by choosing to allocate more memory. It seems more intuitive to simply allocate memory on the heap until we run out or the process is killed. Using jmemnobs.c follows exactly this approach. It has no memory limit and will allocate memory until it runs out or is killed. Another alternative would be to modify jmem-android to simply error exit once we reach a maximum memory limit. This would make behavior more predicatble, but may prevent us from decoding images that could possibly be decoded. This would also bring up the question of how we choose this maximum memory limit. BUG:20541233 Change-Id: I242eff3273e549f9094830d42eb400a8a7810bc9 (cherry picked from commit c4c67ec5d3473d6bfe5b6dc3cfecd5c016f7d2cf)
1 parent 135f5dc commit 19a6799

File tree

3 files changed

+4
-370
lines changed

3 files changed

+4
-370
lines changed

Android.mk

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -11,21 +11,11 @@ LOCAL_SRC_FILES := \
1111
jdinput.c jdmainct.c jdmarker.c jdmaster.c jdmerge.c jdphuff.c \
1212
jdpostct.c jdsample.c jdtrans.c jerror.c jfdctflt.c jfdctfst.c \
1313
jfdctint.c jidctflt.c jidctfst.c jidctint.c jidctred.c jquant1.c \
14-
jquant2.c jutils.c jmemmgr.c armv6_idct.S
14+
jquant2.c jutils.c jmemmgr.c jmemnobs.c armv6_idct.S
1515

16-
ifeq (,$(TARGET_BUILD_APPS))
17-
# building against master
18-
# use ashmem as libjpeg decoder's backing store
19-
LOCAL_CFLAGS += -DUSE_ANDROID_ASHMEM
20-
LOCAL_SRC_FILES += \
21-
jmem-ashmem.c
22-
else
23-
# unbundled branch, built against NDK.
24-
LOCAL_SDK_VERSION := 17
25-
# the original android memory manager.
26-
# use sdcard as libjpeg decoder's backing store
27-
LOCAL_SRC_FILES += \
28-
jmem-android.c
16+
ifneq (,$(TARGET_BUILD_APPS))
17+
# unbundled branch, built against NDK.
18+
LOCAL_SDK_VERSION := 17
2919
endif
3020

3121
LOCAL_CFLAGS += -DAVOID_TABLES

jmem-android.c

Lines changed: 0 additions & 186 deletions
This file was deleted.

jmem-ashmem.c

Lines changed: 0 additions & 170 deletions
This file was deleted.

0 commit comments

Comments
 (0)