diff options
author | Elliott Hughes <enh@google.com> | 2015-01-20 18:09:05 -0800 |
---|---|---|
committer | Elliott Hughes <enh@google.com> | 2015-01-21 10:33:30 -0800 |
commit | 8c4994bbc1a9a01e34ea92c91eb5b2d1a27bd074 (patch) | |
tree | 8b632cea0832373b9cb843427bb5976b3668f1a2 | |
parent | f374358414812d3e5a45ba75a2b1926693924420 (diff) | |
download | bionic-8c4994bbc1a9a01e34ea92c91eb5b2d1a27bd074.zip bionic-8c4994bbc1a9a01e34ea92c91eb5b2d1a27bd074.tar.gz bionic-8c4994bbc1a9a01e34ea92c91eb5b2d1a27bd074.tar.bz2 |
Implement __fsetlocking.
The old __isthreaded hack was never very useful on Android because all user
code runs in a VM where there are lots of threads running. But __fsetlocking
lets a caller say "I'll worry about the locking for this FILE*", which is
useful for the normal case where you don't share a FILE* between threads
so you don't need any locking.
Bug: 17154740
Bug: 18593728
Change-Id: I2a8dddc29d3edff39a3d7d793387f2253608a68d
-rw-r--r-- | benchmarks/stdio_benchmark.cpp | 16 | ||||
-rw-r--r-- | libc/stdio/fileext.h | 9 | ||||
-rw-r--r-- | libc/stdio/local.h | 4 | ||||
-rw-r--r-- | libc/stdio/stdio_ext.cpp | 25 | ||||
-rw-r--r-- | tests/stdio_ext_test.cpp | 5 |
5 files changed, 41 insertions, 18 deletions
diff --git a/benchmarks/stdio_benchmark.cpp b/benchmarks/stdio_benchmark.cpp index fe25d76..5658a50 100644 --- a/benchmarks/stdio_benchmark.cpp +++ b/benchmarks/stdio_benchmark.cpp @@ -17,6 +17,7 @@ #include "benchmark.h" #include <stdio.h> +#include <stdio_ext.h> #define KB 1024 #define MB 1024*KB @@ -29,6 +30,7 @@ template <typename Fn> static void ReadWriteTest(int iters, int chunk_size, Fn f, bool buffered) { StopBenchmarkTiming(); FILE* fp = fopen("/dev/zero", "rw"); + __fsetlocking(fp, FSETLOCKING_BYCALLER); char* buf = new char[chunk_size]; StartBenchmarkTiming(); @@ -66,12 +68,22 @@ static void BM_stdio_fwrite_unbuffered(int iters, int chunk_size) { } BENCHMARK(BM_stdio_fwrite_unbuffered)->AT_COMMON_SIZES; -static void BM_stdio_fopen_fgets_fclose(int iters) { +static void FopenFgetsFclose(int iters, bool no_locking) { char buf[1024]; for (int i = 0; i < iters; ++i) { FILE* fp = fopen("/proc/version", "re"); + if (no_locking) __fsetlocking(fp, FSETLOCKING_BYCALLER); fgets(buf, sizeof(buf), fp); fclose(fp); } } -BENCHMARK(BM_stdio_fopen_fgets_fclose); + +static void BM_stdio_fopen_fgets_fclose_locking(int iters) { + FopenFgetsFclose(iters, false); +} +BENCHMARK(BM_stdio_fopen_fgets_fclose_locking); + +static void BM_stdio_fopen_fgets_fclose_no_locking(int iters) { + FopenFgetsFclose(iters, true); +} +BENCHMARK(BM_stdio_fopen_fgets_fclose_no_locking); diff --git a/libc/stdio/fileext.h b/libc/stdio/fileext.h index 25b7bda..75230cd 100644 --- a/libc/stdio/fileext.h +++ b/libc/stdio/fileext.h @@ -33,6 +33,7 @@ #define _FILEEXT_H_ #include <pthread.h> +#include <stdbool.h> __BEGIN_DECLS @@ -40,9 +41,10 @@ __BEGIN_DECLS * file extension */ struct __sfileext { - struct __sbuf _ub; /* ungetc buffer */ + struct __sbuf _ub; /* ungetc buffer */ struct wchar_io_data _wcio; /* wide char io status */ - pthread_mutex_t _lock; /* file lock */ + pthread_mutex_t _lock; /* file lock */ + bool _stdio_handles_locking; /* __fsetlocking support */ }; #define _EXT(fp) ((struct __sfileext *)((fp)->_ext._base)) @@ -54,7 +56,8 @@ do { \ _UB(fp)._base = NULL; \ _UB(fp)._size = 0; \ WCIO_INIT(fp); \ - _FLOCK(fp).value = __PTHREAD_RECURSIVE_MUTEX_INIT_VALUE; \ + _FLOCK(fp).value = __PTHREAD_RECURSIVE_MUTEX_INIT_VALUE; \ + _EXT(fp)->_stdio_handles_locking = true; \ } while (0) #define _FILEEXT_SETUP(f, fext) \ diff --git a/libc/stdio/local.h b/libc/stdio/local.h index 46b11f1..7033eda 100644 --- a/libc/stdio/local.h +++ b/libc/stdio/local.h @@ -111,8 +111,8 @@ extern void __atexit_register_cleanup(void (*)(void)); (fp)->_lb._base = NULL; \ } -#define FLOCKFILE(fp) flockfile(fp) -#define FUNLOCKFILE(fp) funlockfile(fp) +#define FLOCKFILE(fp) if (_EXT(fp)->_stdio_handles_locking) flockfile(fp) +#define FUNLOCKFILE(fp) if (_EXT(fp)->_stdio_handles_locking) funlockfile(fp) #define FLOATING_POINT #define PRINTF_WIDE_CHAR diff --git a/libc/stdio/stdio_ext.cpp b/libc/stdio/stdio_ext.cpp index bfdecb8..fea44f6 100644 --- a/libc/stdio/stdio_ext.cpp +++ b/libc/stdio/stdio_ext.cpp @@ -27,13 +27,10 @@ */ #include <stdio_ext.h> +#include <stdlib.h> -#include <stdio.h> #include "local.h" - -#define FSETLOCKING_QUERY 0 -#define FSETLOCKING_INTERNAL 1 -#define FSETLOCKING_BYCALLER 2 +#include "private/libc_logging.h" size_t __fbufsize(FILE* fp) { return fp->_bf._size; @@ -76,11 +73,19 @@ void _flushlbf() { fflush(NULL); } -int __fsetlocking(FILE*, int) { - // We don't currently have an implementation that would obey this, - // so make setting the state a no-op and always return "we handle locking for you". - // http://b/17154740 suggests ways we could fix this. - return FSETLOCKING_INTERNAL; +int __fsetlocking(FILE* fp, int type) { + int old_state = _EXT(fp)->_stdio_handles_locking ? FSETLOCKING_INTERNAL : FSETLOCKING_BYCALLER; + if (type == FSETLOCKING_QUERY) { + return old_state; + } + + if (type != FSETLOCKING_INTERNAL && type != FSETLOCKING_BYCALLER) { + // The API doesn't let us report an error, so blow up. + __libc_fatal("Bad type (%d) passed to __fsetlocking", type); + } + + _EXT(fp)->_stdio_handles_locking = (type == FSETLOCKING_INTERNAL); + return old_state; } void clearerr_unlocked(FILE* fp) { diff --git a/tests/stdio_ext_test.cpp b/tests/stdio_ext_test.cpp index 950c7ed..c95cbbd 100644 --- a/tests/stdio_ext_test.cpp +++ b/tests/stdio_ext_test.cpp @@ -133,7 +133,10 @@ TEST(stdio_ext, __freadable__fwritable) { TEST(stdio_ext, __fsetlocking) { FILE* fp = fopen("/proc/version", "r"); - // Android doesn't actually support the other modes. + ASSERT_EQ(FSETLOCKING_INTERNAL, __fsetlocking(fp, FSETLOCKING_QUERY)); + ASSERT_EQ(FSETLOCKING_INTERNAL, __fsetlocking(fp, FSETLOCKING_BYCALLER)); + ASSERT_EQ(FSETLOCKING_BYCALLER, __fsetlocking(fp, FSETLOCKING_QUERY)); + ASSERT_EQ(FSETLOCKING_BYCALLER, __fsetlocking(fp, FSETLOCKING_INTERNAL)); ASSERT_EQ(FSETLOCKING_INTERNAL, __fsetlocking(fp, FSETLOCKING_QUERY)); fclose(fp); } |