Cache getpid.
authorElliott Hughes <enh@google.com>
Thu, 19 Jun 2014 23:39:01 +0000 (16:39 -0700)
committerElliott Hughes <enh@google.com>
Fri, 20 Jun 2014 16:06:57 +0000 (09:06 -0700)
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

15 files changed:
benchmarks/unistd_benchmark.cpp
libc/Android.mk
libc/SYSCALLS.TXT
libc/arch-arm/syscalls/__getpid.S [moved from libc/arch-arm/syscalls/getpid.S with 89% similarity]
libc/arch-arm64/syscalls/__getpid.S [moved from libc/arch-arm64/syscalls/getpid.S with 82% similarity]
libc/arch-mips/syscalls/__getpid.S [moved from libc/arch-mips/syscalls/getpid.S with 89% similarity]
libc/arch-mips64/syscalls/__getpid.S [moved from libc/arch-mips64/syscalls/getpid.S with 87% similarity]
libc/arch-x86/syscalls/__getpid.S [moved from libc/arch-x86/syscalls/getpid.S with 89% similarity]
libc/arch-x86_64/syscalls/__getpid.S [moved from libc/arch-x86_64/syscalls/getpid.S with 83% similarity]
libc/bionic/fork.cpp
libc/bionic/getpid.cpp [new file with mode: 0644]
libc/bionic/libc_init_common.cpp
libc/bionic/pthread_create.cpp
libc/bionic/pthread_internal.h
tests/unistd_test.cpp

index c35e7c369f95df9bf0653caf0e43c115ebff7439..7e2ac30c920641587059b07ae7ccacccd652aef6 100644 (file)
@@ -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;
 
index a6298a0ad3ac95ae63e12c25eca76e1b8d27f53b..002978c4e26707bd440fdf32649a9e6dc8783249 100644 (file)
@@ -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 \
index 369b23c76566c4e0b317be52f1117671370f563f..839cfb7f470d653a6533e9ff9f6884add33a2907 100644 (file)
@@ -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
similarity index 89%
rename from libc/arch-arm/syscalls/getpid.S
rename to libc/arch-arm/syscalls/__getpid.S
index 10bcb8e28b0bd65a3dea0332f76c8a0b0d056f9b..eedc33a5f3730aeded4871dbf8858260f3ea2c1f 100644 (file)
@@ -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)
similarity index 82%
rename from libc/arch-arm64/syscalls/getpid.S
rename to libc/arch-arm64/syscalls/__getpid.S
index 1802ce80a17d78cc6b02cff49c7b688a7662e762..c3003c3024876eb6cfe408531f7822d880f477c5 100644 (file)
@@ -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
similarity index 89%
rename from libc/arch-mips/syscalls/getpid.S
rename to libc/arch-mips/syscalls/__getpid.S
index a053f5be0af08c8841b0dd8da6e1cc4d3d086a07..52cf6f426faa43ce2c31b00d68c09b31d9f27818 100644 (file)
@@ -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)
similarity index 87%
rename from libc/arch-mips64/syscalls/getpid.S
rename to libc/arch-mips64/syscalls/__getpid.S
index 3b457b532df96cb1cdc2fdee73022c36f964f3d2..0977ff023103178f772b8fec5c1071c16badc3fb 100644 (file)
@@ -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
similarity index 89%
rename from libc/arch-x86/syscalls/getpid.S
rename to libc/arch-x86/syscalls/__getpid.S
index 0e12d5a3ef0859cba8f254c0995e32a75de2b1df..f138d2f8814ed444a14e888832bfef59c67ed6aa 100644 (file)
@@ -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)
similarity index 83%
rename from libc/arch-x86_64/syscalls/getpid.S
rename to libc/arch-x86_64/syscalls/__getpid.S
index a2d732c9ef7952acce6e97806926faf13e3c4b74..bd1bf1ee1cb2b0cdee221d52cf896156b63a7068 100644 (file)
@@ -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
index a0f98e42f307d01aa617263a71c65241537df630..6cfc736649889ac1d6a363195e15bfcf0cbdce9d 100644 (file)
 
 #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 (file)
index 0000000..a3d5b35
--- /dev/null
@@ -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();
+}
index abf2d3674248a595d7cc8857fe0d426a937dfe02..fa61c3c1090ca4f06ed888dc190562b571d327c7 100644 (file)
@@ -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);
index c83b5a0eebc44419ede04f8925d4c9e09a7b5fa6..2ded22b0f339e04a04a0558ed84cda8c1b09d993 100644 (file)
@@ -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;
index 490ae86d4633a5448f8bda44afd4b12528ba4571..e05d15c79cdba05c1bc8ee8510da25f39b8db5ba 100644 (file)
@@ -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;
index 6aa259afb56ba595f006e29af31742e66dd4e3c7..95e63b36835195f7ce6c1098873d88e43c41b6c5 100644 (file)
@@ -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);
+}