diff options
author | Elliott Hughes <enh@google.com> | 2014-06-19 16:39:01 -0700 |
---|---|---|
committer | Elliott Hughes <enh@google.com> | 2014-06-20 09:06:57 -0700 |
commit | 7086ad6919feb2415c6027163f5c63323bcca27c (patch) | |
tree | f32c06d513e90ba1fcb8fe56f9cd055cccc7d01b | |
parent | d1bf37780d0bcaca3e6046171f958ebfea34bde1 (diff) | |
download | bionic-7086ad6919feb2415c6027163f5c63323bcca27c.zip bionic-7086ad6919feb2415c6027163f5c63323bcca27c.tar.gz bionic-7086ad6919feb2415c6027163f5c63323bcca27c.tar.bz2 |
Cache getpid.
In practice, with this implementation we never need to make a system call.
We get the main thread's tid (which is the same as our pid) back from
the set_tid_address system call we have to make during initialization.
A new pthread will have the same pid as its parent, and a fork child's
main (and only) thread will have a pid equal to its tid, which we get for
free from the kernel before clone returns.
The only time we'd actually have to make a getpid system call now is if
we take a signal during fork and the signal handler calls getpid. (That,
or we call getpid in the dynamic linker while it's still dealing with its
own relocations and hasn't even set up the main thread yet.)
Bug: 15387103
Change-Id: I6d4718ed0a5c912fc75b5f738c49a023dbed5189
-rw-r--r-- | benchmarks/unistd_benchmark.cpp | 11 | ||||
-rw-r--r-- | libc/Android.mk | 1 | ||||
-rw-r--r-- | libc/SYSCALLS.TXT | 2 | ||||
-rw-r--r-- | libc/arch-arm/syscalls/__getpid.S (renamed from libc/arch-arm/syscalls/getpid.S) | 4 | ||||
-rw-r--r-- | libc/arch-arm64/syscalls/__getpid.S (renamed from libc/arch-arm64/syscalls/getpid.S) | 5 | ||||
-rw-r--r-- | libc/arch-mips/syscalls/__getpid.S (renamed from libc/arch-mips/syscalls/getpid.S) | 4 | ||||
-rw-r--r-- | libc/arch-mips64/syscalls/__getpid.S (renamed from libc/arch-mips64/syscalls/getpid.S) | 5 | ||||
-rw-r--r-- | libc/arch-x86/syscalls/__getpid.S (renamed from libc/arch-x86/syscalls/getpid.S) | 4 | ||||
-rw-r--r-- | libc/arch-x86_64/syscalls/__getpid.S (renamed from libc/arch-x86_64/syscalls/getpid.S) | 5 | ||||
-rw-r--r-- | libc/bionic/fork.cpp | 13 | ||||
-rw-r--r-- | libc/bionic/getpid.cpp | 47 | ||||
-rw-r--r-- | libc/bionic/libc_init_common.cpp | 17 | ||||
-rw-r--r-- | libc/bionic/pthread_create.cpp | 3 | ||||
-rw-r--r-- | libc/bionic/pthread_internal.h | 20 | ||||
-rw-r--r-- | tests/unistd_test.cpp | 45 |
15 files changed, 163 insertions, 23 deletions
diff --git a/benchmarks/unistd_benchmark.cpp b/benchmarks/unistd_benchmark.cpp index c35e7c3..7e2ac30 100644 --- a/benchmarks/unistd_benchmark.cpp +++ b/benchmarks/unistd_benchmark.cpp @@ -30,6 +30,17 @@ static void BM_unistd_getpid(int iters) { } BENCHMARK(BM_unistd_getpid); +static void BM_unistd_getpid_syscall(int iters) { + StartBenchmarkTiming(); + + for (int i = 0; i < iters; ++i) { + syscall(__NR_getpid); + } + + StopBenchmarkTiming(); +} +BENCHMARK(BM_unistd_getpid_syscall); + // Stop GCC optimizing out our pure function. /* Must not be static! */ pid_t (*gettid_fp)() = gettid; diff --git a/libc/Android.mk b/libc/Android.mk index a6298a0..002978c 100644 --- a/libc/Android.mk +++ b/libc/Android.mk @@ -127,6 +127,7 @@ libc_bionic_src_files := \ bionic/getauxval.cpp \ bionic/getcwd.cpp \ bionic/getpgrp.cpp \ + bionic/getpid.cpp \ bionic/gettid.cpp \ bionic/inotify_init.cpp \ bionic/lchown.cpp \ diff --git a/libc/SYSCALLS.TXT b/libc/SYSCALLS.TXT index 369b23c..839cfb7 100644 --- a/libc/SYSCALLS.TXT +++ b/libc/SYSCALLS.TXT @@ -97,7 +97,7 @@ ssize_t pread64|pread(int, void*, size_t, off_t) arm64,mips64,x86_64 ssize_t pwrite64(int, void*, size_t, off64_t) arm,mips,x86 ssize_t pwrite64|pwrite(int, void*, size_t, off_t) arm64,mips64,x86_64 int close(int) all -pid_t getpid() all +pid_t __getpid:getpid() all int munmap(void*, size_t) all void* mremap(void*, size_t, size_t, unsigned long) all int msync(const void*, size_t, int) all diff --git a/libc/arch-arm/syscalls/getpid.S b/libc/arch-arm/syscalls/__getpid.S index 10bcb8e..eedc33a 100644 --- a/libc/arch-arm/syscalls/getpid.S +++ b/libc/arch-arm/syscalls/__getpid.S @@ -2,7 +2,7 @@ #include <private/bionic_asm.h> -ENTRY(getpid) +ENTRY(__getpid) mov ip, r7 ldr r7, =__NR_getpid swi #0 @@ -11,4 +11,4 @@ ENTRY(getpid) bxls lr neg r0, r0 b __set_errno -END(getpid) +END(__getpid) diff --git a/libc/arch-arm64/syscalls/getpid.S b/libc/arch-arm64/syscalls/__getpid.S index 1802ce8..c3003c3 100644 --- a/libc/arch-arm64/syscalls/getpid.S +++ b/libc/arch-arm64/syscalls/__getpid.S @@ -2,7 +2,7 @@ #include <private/bionic_asm.h> -ENTRY(getpid) +ENTRY(__getpid) mov x8, __NR_getpid svc #0 @@ -11,4 +11,5 @@ ENTRY(getpid) b.hi __set_errno ret -END(getpid) +END(__getpid) +.hidden __getpid diff --git a/libc/arch-mips/syscalls/getpid.S b/libc/arch-mips/syscalls/__getpid.S index a053f5b..52cf6f4 100644 --- a/libc/arch-mips/syscalls/getpid.S +++ b/libc/arch-mips/syscalls/__getpid.S @@ -2,7 +2,7 @@ #include <private/bionic_asm.h> -ENTRY(getpid) +ENTRY(__getpid) .set noreorder .cpload t9 li v0, __NR_getpid @@ -16,4 +16,4 @@ ENTRY(getpid) j t9 nop .set reorder -END(getpid) +END(__getpid) diff --git a/libc/arch-mips64/syscalls/getpid.S b/libc/arch-mips64/syscalls/__getpid.S index 3b457b5..0977ff0 100644 --- a/libc/arch-mips64/syscalls/getpid.S +++ b/libc/arch-mips64/syscalls/__getpid.S @@ -2,7 +2,7 @@ #include <private/bionic_asm.h> -ENTRY(getpid) +ENTRY(__getpid) .set push .set noreorder li v0, __NR_getpid @@ -22,4 +22,5 @@ ENTRY(getpid) j t9 move ra, t0 .set pop -END(getpid) +END(__getpid) +.hidden __getpid diff --git a/libc/arch-x86/syscalls/getpid.S b/libc/arch-x86/syscalls/__getpid.S index 0e12d5a..f138d2f 100644 --- a/libc/arch-x86/syscalls/getpid.S +++ b/libc/arch-x86/syscalls/__getpid.S @@ -2,7 +2,7 @@ #include <private/bionic_asm.h> -ENTRY(getpid) +ENTRY(__getpid) movl $__NR_getpid, %eax int $0x80 cmpl $-MAX_ERRNO, %eax @@ -13,4 +13,4 @@ ENTRY(getpid) addl $4, %esp 1: ret -END(getpid) +END(__getpid) diff --git a/libc/arch-x86_64/syscalls/getpid.S b/libc/arch-x86_64/syscalls/__getpid.S index a2d732c..bd1bf1e 100644 --- a/libc/arch-x86_64/syscalls/getpid.S +++ b/libc/arch-x86_64/syscalls/__getpid.S @@ -2,7 +2,7 @@ #include <private/bionic_asm.h> -ENTRY(getpid) +ENTRY(__getpid) movl $__NR_getpid, %eax syscall cmpq $-MAX_ERRNO, %rax @@ -12,4 +12,5 @@ ENTRY(getpid) call __set_errno 1: ret -END(getpid) +END(__getpid) +.hidden __getpid diff --git a/libc/bionic/fork.cpp b/libc/bionic/fork.cpp index a0f98e4..6cfc736 100644 --- a/libc/bionic/fork.cpp +++ b/libc/bionic/fork.cpp @@ -31,19 +31,26 @@ #include "pthread_internal.h" +#define FORK_FLAGS (CLONE_CHILD_SETTID | CLONE_CHILD_CLEARTID | SIGCHLD) + int fork() { __bionic_atfork_run_prepare(); pthread_internal_t* self = __get_thread(); - int flags = CLONE_CHILD_SETTID | CLONE_CHILD_CLEARTID | SIGCHLD; + + // Remember the parent pid and invalidate the cached value while we fork. + pid_t parent_pid = self->invalidate_cached_pid(); + #if defined(__x86_64__) // sys_clone's last two arguments are flipped on x86-64. - int result = syscall(__NR_clone, flags, NULL, NULL, &(self->tid), NULL); + int result = syscall(__NR_clone, FORK_FLAGS, NULL, NULL, &(self->tid), NULL); #else - int result = syscall(__NR_clone, flags, NULL, NULL, NULL, &(self->tid)); + int result = syscall(__NR_clone, FORK_FLAGS, NULL, NULL, NULL, &(self->tid)); #endif if (result == 0) { + self->set_cached_pid(gettid()); __bionic_atfork_run_child(); } else { + self->set_cached_pid(parent_pid); __bionic_atfork_run_parent(); } return result; diff --git a/libc/bionic/getpid.cpp b/libc/bionic/getpid.cpp new file mode 100644 index 0000000..a3d5b35 --- /dev/null +++ b/libc/bionic/getpid.cpp @@ -0,0 +1,47 @@ +/* + * Copyright (C) 2014 The Android Open Source Project + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in + * the documentation and/or other materials provided with the + * distribution. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS + * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE + * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, + * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS + * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED + * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, + * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT + * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF + * SUCH DAMAGE. + */ + +#include <unistd.h> + +#include "pthread_internal.h" + +extern "C" pid_t __getpid(); + +pid_t getpid() { + pthread_internal_t* self = __get_thread(); + + // Do we have a valid cached pid? + pid_t cached_pid; + if (__predict_true(self->get_cached_pid(&cached_pid))) { + return cached_pid; + } + + // We're still in the dynamic linker or we're in the middle of forking, so ask the kernel. + // We don't know whether it's safe to update the cached value, so don't try. + return __getpid(); +} diff --git a/libc/bionic/libc_init_common.cpp b/libc/bionic/libc_init_common.cpp index abf2d36..fa61c3c 100644 --- a/libc/bionic/libc_init_common.cpp +++ b/libc/bionic/libc_init_common.cpp @@ -86,21 +86,24 @@ static size_t get_main_thread_stack_size() { void __libc_init_tls(KernelArgumentBlock& args) { __libc_auxv = args.auxv; - uintptr_t stack_top = (__get_sp() & ~(PAGE_SIZE - 1)) + PAGE_SIZE; - size_t stack_size = get_main_thread_stack_size(); - uintptr_t stack_bottom = stack_top - stack_size; - static void* tls[BIONIC_TLS_SLOTS]; static pthread_internal_t main_thread; main_thread.tls = tls; // Tell the kernel to clear our tid field when we exit, so we're like any other pthread. + // As a side-effect, this tells us our pid (which is the same as the main thread's tid). main_thread.tid = __set_tid_address(&main_thread.tid); + main_thread.set_cached_pid(main_thread.tid); + + // Work out the extent of the main thread's stack. + uintptr_t stack_top = (__get_sp() & ~(PAGE_SIZE - 1)) + PAGE_SIZE; + size_t stack_size = get_main_thread_stack_size(); + void* stack_bottom = reinterpret_cast<void*>(stack_top - stack_size); - // We already have a stack, and we don't want to free it up on exit (because things like - // environment variables with global scope live on it). + // We don't want to free the main thread's stack even when the main thread exits + // because things like environment variables with global scope live on it. pthread_attr_init(&main_thread.attr); - pthread_attr_setstack(&main_thread.attr, (void*) stack_bottom, stack_size); + pthread_attr_setstack(&main_thread.attr, stack_bottom, stack_size); main_thread.attr.flags = PTHREAD_ATTR_FLAG_USER_ALLOCATED_STACK | PTHREAD_ATTR_FLAG_MAIN_THREAD; __init_thread(&main_thread, false); diff --git a/libc/bionic/pthread_create.cpp b/libc/bionic/pthread_create.cpp index c83b5a0..2ded22b 100644 --- a/libc/bionic/pthread_create.cpp +++ b/libc/bionic/pthread_create.cpp @@ -30,6 +30,7 @@ #include <errno.h> #include <sys/mman.h> +#include <unistd.h> #include "pthread_internal.h" @@ -220,6 +221,8 @@ int pthread_create(pthread_t* thread_out, pthread_attr_t const* attr, thread->start_routine = start_routine; thread->start_routine_arg = arg; + thread->set_cached_pid(getpid()); + int flags = CLONE_VM | CLONE_FS | CLONE_FILES | CLONE_SIGHAND | CLONE_THREAD | CLONE_SYSVSEM | CLONE_SETTLS | CLONE_PARENT_SETTID | CLONE_CHILD_CLEARTID; void* tls = thread->tls; diff --git a/libc/bionic/pthread_internal.h b/libc/bionic/pthread_internal.h index 490ae86..e05d15c 100644 --- a/libc/bionic/pthread_internal.h +++ b/libc/bionic/pthread_internal.h @@ -36,6 +36,26 @@ struct pthread_internal_t { pid_t tid; + private: + pid_t cached_pid_; + + public: + pid_t invalidate_cached_pid() { + pid_t old_value; + get_cached_pid(&old_value); + set_cached_pid(0); + return old_value; + } + + void set_cached_pid(pid_t value) { + cached_pid_ = value; + } + + bool get_cached_pid(pid_t* cached_pid) { + *cached_pid = cached_pid_; + return (*cached_pid != 0); + } + void** tls; pthread_attr_t attr; diff --git a/tests/unistd_test.cpp b/tests/unistd_test.cpp index 6aa259a..95e63b3 100644 --- a/tests/unistd_test.cpp +++ b/tests/unistd_test.cpp @@ -22,6 +22,7 @@ #include <fcntl.h> #include <stdint.h> #include <unistd.h> +#include <sys/syscall.h> #include <sys/types.h> #include <sys/wait.h> @@ -379,3 +380,47 @@ TEST(unistd, fdatasync) { TEST(unistd, fsync) { TestFsyncFunction(fsync); } + +TEST(unistd, getpid_caching_and_fork) { + pid_t parent_pid = getpid(); + ASSERT_EQ(syscall(__NR_getpid), parent_pid); + + pid_t fork_result = fork(); + ASSERT_NE(fork_result, -1); + if (fork_result == 0) { + // We're the child. + ASSERT_EQ(syscall(__NR_getpid), getpid()); + ASSERT_EQ(parent_pid, getppid()); + _exit(123); + } else { + // We're the parent. + ASSERT_EQ(parent_pid, getpid()); + + int status; + ASSERT_EQ(fork_result, waitpid(fork_result, &status, 0)); + ASSERT_TRUE(WIFEXITED(status)); + ASSERT_EQ(123, WEXITSTATUS(status)); + } +} + +static void GetPidCachingHelperHelper() { + ASSERT_EQ(syscall(__NR_getpid), getpid()); +} + +static void* GetPidCachingHelper(void*) { + GetPidCachingHelperHelper(); // Can't assert in a non-void function. + return NULL; +} + +TEST(unistd, getpid_caching_and_pthread_create) { + pid_t parent_pid = getpid(); + + pthread_t t; + ASSERT_EQ(0, pthread_create(&t, NULL, GetPidCachingHelper, NULL)); + + ASSERT_EQ(parent_pid, getpid()); + + void* result; + ASSERT_EQ(0, pthread_join(t, &result)); + ASSERT_EQ(NULL, result); +} |