From 82e97442fa38d7aad60fbc19a0df811cef4fbdf9 Mon Sep 17 00:00:00 2001 From: Daniel Colascione Date: Tue, 3 Sep 2019 13:42:23 -0700 Subject: [PATCH] Make android_os_Process_readProcFile allocate less Make android_os_Process_readProcFile read small proc files into a stack-allocated buffer and fall back to the heap only for larger files. Avoid pointless zeroing of memory that we're going to use for read() buffers. Read proc files atomically instead of piece-by-piece. Use Android RAII helpers instead of explicit resource management. Bug: 140439982 Test: booted, works Change-Id: Ifc02a4bbab0cfd968aff841d7ce0af846db613eb --- core/jni/android_util_Process.cpp | 83 +++++++++++++++++++++---------- 1 file changed, 57 insertions(+), 26 deletions(-) diff --git a/core/jni/android_util_Process.cpp b/core/jni/android_util_Process.cpp index 2d7069c50255e..a52a7ed982ecb 100644 --- a/core/jni/android_util_Process.cpp +++ b/core/jni/android_util_Process.cpp @@ -28,7 +28,11 @@ #include #include #include +#include +#include +#include +#include #include #include @@ -58,8 +62,18 @@ using namespace android; static const bool kDebugPolicy = false; static const bool kDebugProc = false; -// When reading `proc` files, how many bytes to read at a time -static const int kReadSize = 4096; + +// Stack reservation for reading small proc files. Most callers of +// readProcFile() are reading files under this threshold, e.g., +// /proc/pid/stat. /proc/pid/time_in_state ends up being about 520 +// bytes, so use 1024 for the stack to provide a bit of slack. +static const ssize_t kProcReadStackBufferSize = 1024; + +// The other files we read from proc tend to be a bit larger (e.g., +// /proc/stat is about 3kB), so once we exhaust the stack buffer, +// retry with a relatively large heap-allocated buffer. We double +// this size and retry until the whole file fits. +static const ssize_t kProcReadMinHeapBufferSize = 4096; #if GUARD_THREAD_PRIORITY Mutex gKeyCreateMutex; @@ -1004,9 +1018,9 @@ jboolean android_os_Process_readProcFile(JNIEnv* env, jobject clazz, jniThrowException(env, "java/lang/OutOfMemoryError", NULL); return JNI_FALSE; } - int fd = open(file8, O_RDONLY | O_CLOEXEC); - if (fd < 0) { + ::android::base::unique_fd fd(open(file8, O_RDONLY | O_CLOEXEC)); + if (!fd.ok()) { if (kDebugProc) { ALOGW("Unable to open process file: %s\n", file8); } @@ -1015,35 +1029,52 @@ jboolean android_os_Process_readProcFile(JNIEnv* env, jobject clazz, } env->ReleaseStringUTFChars(file, file8); - std::vector fileBuffer(kReadSize); - int numBytesRead = 0; - while (true) { - // Resize buffer to make space for contents. This might be more than we need, but once we've - // read we resize back down - fileBuffer.resize(numBytesRead + kReadSize, 0); - // Read in contents - int len = TEMP_FAILURE_RETRY(read(fd, fileBuffer.data() + numBytesRead, kReadSize)); - numBytesRead += len; - if (len < 0) { - // If `len` is negative, an error occurred on read + // Most proc files we read are small, so we only go through the + // loop once and use the stack buffer. We allocate a buffer big + // enough for the whole file. + + char readBufferStack[kProcReadStackBufferSize]; + std::unique_ptr readBufferHeap; + char* readBuffer = &readBufferStack[0]; + ssize_t readBufferSize = kProcReadStackBufferSize; + ssize_t numberBytesRead; + for (;;) { + // By using pread, we can avoid an lseek to rewind the FD + // before retry, saving a system call. + numberBytesRead = pread(fd, readBuffer, readBufferSize, 0); + if (numberBytesRead < 0 && errno == EINTR) { + continue; + } + if (numberBytesRead < 0) { if (kDebugProc) { - ALOGW("Unable to open process file: %s fd=%d\n", file8, fd); + ALOGW("Unable to open process file: %s fd=%d\n", file8, fd.get()); } - close(fd); return JNI_FALSE; - } else if (len == 0) { - // If nothing read, we're done + } + if (numberBytesRead < readBufferSize) { break; } + if (readBufferSize > std::numeric_limits::max() / 2) { + if (kDebugProc) { + ALOGW("Proc file too big: %s fd=%d\n", file8, fd.get()); + } + return JNI_FALSE; + } + readBufferSize = std::max(readBufferSize * 2, + kProcReadMinHeapBufferSize); + readBufferHeap.reset(); // Free address space before getting more. + readBufferHeap = std::make_unique(readBufferSize); + if (!readBufferHeap) { + jniThrowException(env, "java/lang/OutOfMemoryError", NULL); + return JNI_FALSE; + } + readBuffer = readBufferHeap.get(); } - // Resize back down to the amount we read - fileBuffer.resize(numBytesRead); - // Terminate buffer with null byte - fileBuffer.push_back('\0'); - close(fd); - return android_os_Process_parseProcLineArray(env, clazz, fileBuffer.data(), 0, numBytesRead, - format, outStrings, outLongs, outFloats); + // parseProcLineArray below modifies the buffer while parsing! + return android_os_Process_parseProcLineArray( + env, clazz, readBuffer, 0, numberBytesRead, + format, outStrings, outLongs, outFloats); } void android_os_Process_setApplicationObject(JNIEnv* env, jobject clazz,