patch: fix and re-enable curthread hash lookup
Arne H. Juul
arnej at pvv.ntnu.no
Mon Feb 26 06:12:13 UTC 2007
On Sun, 25 Feb 2007, Kurt Miller wrote:
> On Thursday 22 February 2007 6:10 pm, Arne H. Juul wrote:
>> This patch is still experimental, so if people can take a look at it and
>> tell me about any problems they can spot that would be much appreciated.
>
> Hi,
>
> I tried to apply the diff but all parts were rejected.
> Can you send this again in unified format? I don't mind
> applying it manually but I only grock unified diffs. :-)
actually I made unified diff first, and found that the important
parts were too unreadable there, that's why I made a context diff...
it *should* apply cleanly to 1.5.0 SCSL sources + BSD patchkit 4,
here it is again in unified format:
diff -ru jdk-1_5_0-scsl.b4/hotspot/src/os/bsd/vm/os_bsd.cpp jdk-1_5_0-scsl.b4.fct2/hotspot/src/os/bsd/vm/os_bsd.cpp
--- jdk-1_5_0-scsl.b4/hotspot/src/os/bsd/vm/os_bsd.cpp 2007-01-24 11:12:39.000000000 +0100
+++ jdk-1_5_0-scsl.b4.fct2/hotspot/src/os/bsd/vm/os_bsd.cpp 2007-02-22 23:53:00.000000000 +0100
@@ -138,15 +138,39 @@
// thread value in thread local storage.
#endif
+ uintptr_t raw = pd_raw_thread_id();
+ int ix = pd_cache_index(raw);
+
+ if (thread != NULL) {
+ // first make sure that nobody gets a cache collision pointing to
+ // this thread by updating its raw id
+ thread->_self_raw_id = raw;
+
// Store the new value before updating the cache to prevent a race
// between get_thread_via_cache_slowly() and this store operation.
os::thread_local_storage_at_put(ThreadLocalStorage::thread_index(), thread);
- // Update thread cache with new thread if setting on thread create,
- // or NO_CACHED_THREAD (zeroed) thread if resetting thread on exit.
- uintptr_t raw = pd_raw_thread_id();
- int ix = pd_cache_index(raw);
- _get_thread_cache[ix] = thread == NULL ? NO_CACHED_THREAD : thread;
+ // Update thread cache with new thread if setting on thread create
+ _get_thread_cache[ix] = thread;
+ } else {
+ // deleting current thread, must get old value first
+ Thread* was_thread = Thread::current();
+
+ // first make sure that nobody gets a cache collision pointing to
+ // the old thread data (soon to be free'd) by updating the raw id
+ was_thread->_self_raw_id = raw;
+
+ // again, update the "real" value first:
+ os::thread_local_storage_at_put(ThreadLocalStorage::thread_index(), thread);
+
+ // then invalidate all cache elements that pointed to this thread
+ // by setting them to NO_CACHED_THREAD (zeroed thread)
+ for (int ix = 0; ix < _pd_cache_size; ++ix) {
+ if (_get_thread_cache[ix] == was_thread) {
+ _get_thread_cache[ix] = NO_CACHED_THREAD;
+ }
+ }
+ }
}
void ThreadLocalStorage::pd_init() {
@@ -1190,7 +1214,7 @@
// XXXBSD: hmm... really do not need?
void os::free_thread_local_storage(int index) {
// %%% don't think we need anything here
- // if ( pthread_key_delete((pthread_key_t) tk) )
+ // if ( pthread_key_delete((pthread_key_t) index) )
// fatal("os::free_thread_local_storage: pthread_key_delete failed");
}
diff -ru jdk-1_5_0-scsl.b4/hotspot/src/os/bsd/vm/thread_bsd.inline.hpp jdk-1_5_0-scsl.b4.fct2/hotspot/src/os/bsd/vm/thread_bsd.inline.hpp
--- jdk-1_5_0-scsl.b4/hotspot/src/os/bsd/vm/thread_bsd.inline.hpp 2007-01-24 11:12:39.000000000 +0100
+++ jdk-1_5_0-scsl.b4.fct2/hotspot/src/os/bsd/vm/thread_bsd.inline.hpp 2007-02-22 23:53:00.000000000 +0100
@@ -19,18 +19,31 @@
uintptr_t raw = pd_raw_thread_id();
int ix = pd_cache_index(raw);
-// XXXBSD: disable fast case. there is a race condition where the
-// fast case returns a different thread from the slow case and has
-// been seen on both OpenBSD and FreeBSD.
+// XXXBSD: fast case.
+// there was a race condition where the fast case returned a different
+// thread from the slow case and has been seen on both OpenBSD and
+// FreeBSD, but I think it's fixed, so enable it for now:
#if 1
- return ThreadLocalStorage::get_thread_via_cache_slowly(raw, ix);
-#else
Thread *Candidate = ThreadLocalStorage::_get_thread_cache[ix];
+
if (Candidate->_self_raw_id == raw) {
- // hit
+ // direct hit
return Candidate;
- } else {
- return ThreadLocalStorage::get_thread_via_cache_slowly(raw, ix);
}
+
+ address stacktop = Candidate->_stack_base;
+ address stackbot = Candidate->_stack_base - Candidate->_stack_size;
+
+ address sp = pd_sp_address();
+
+ // is this still the right thread? Check if current stack pointer
+ // is within the thread's stack, if ok update it with current raw id.
+ if (stacktop > sp && stackbot <= sp ) {
+ Candidate->_self_raw_id = raw;
+ // indirect hit
+ return Candidate;
+ }
+ ix = pd_cache_index(raw);
#endif
+ return ThreadLocalStorage::get_thread_via_cache_slowly(raw, ix);
}
diff -ru jdk-1_5_0-scsl.b4/hotspot/src/os_cpu/bsd_amd64/vm/threadLS_bsd_amd64.hpp jdk-1_5_0-scsl.b4.fct2/hotspot/src/os_cpu/bsd_amd64/vm/threadLS_bsd_amd64.hpp
--- jdk-1_5_0-scsl.b4/hotspot/src/os_cpu/bsd_amd64/vm/threadLS_bsd_amd64.hpp 2007-01-24 11:12:39.000000000 +0100
+++ jdk-1_5_0-scsl.b4.fct2/hotspot/src/os_cpu/bsd_amd64/vm/threadLS_bsd_amd64.hpp 2007-02-22 23:53:00.000000000 +0100
@@ -9,7 +9,7 @@
// Processor dependent parts of ThreadLocalStorage
private:
- static Thread* _get_thread_cache[]; // index by [(raw_id>>9)^(raw_id>>20) % _pd_cache_size]
+ static Thread* _get_thread_cache[]; // index by [(raw_id^(raw_id>>10)) % _pd_cache_size]
static Thread* get_thread_via_cache_slowly(uintptr_t raw_id, int index);
NOT_PRODUCT(static int _tcacheHit;)
@@ -20,8 +20,8 @@
static void print_statistics() PRODUCT_RETURN;
enum Constants {
- _pd_cache_size = 128*2, // projected typical # of threads * 2
-
+ // projected typical # of threads * 2 * typical active stack pages
+ _pd_cache_size = 128*2*4,
_pd_min_page_size = 4*K,
_pd_typical_stack_size = 512*K
};
@@ -37,9 +37,8 @@
}
static int pd_cache_index(uintptr_t sp_page) {
- return ((sp_page / 2) /* pages tend to come in pairs */
- ^ (sp_page / (_pd_typical_stack_size/_pd_min_page_size)))
- % _pd_cache_size;
+ // _pd_cache_size == 1<<10
+ return (sp_page ^ (sp_page >> 10)) % _pd_cache_size;
}
// Java Thread
diff -ru jdk-1_5_0-scsl.b4/hotspot/src/os_cpu/bsd_i486/vm/threadLS_bsd_i486.hpp jdk-1_5_0-scsl.b4.fct2/hotspot/src/os_cpu/bsd_i486/vm/threadLS_bsd_i486.hpp
--- jdk-1_5_0-scsl.b4/hotspot/src/os_cpu/bsd_i486/vm/threadLS_bsd_i486.hpp 2007-01-24 11:12:39.000000000 +0100
+++ jdk-1_5_0-scsl.b4.fct2/hotspot/src/os_cpu/bsd_i486/vm/threadLS_bsd_i486.hpp 2007-02-22 23:53:00.000000000 +0100
@@ -9,7 +9,7 @@
// Processor dependent parts of ThreadLocalStorage
private:
- static Thread* _get_thread_cache[]; // index by [(raw_id>>9)^(raw_id>>20) % _pd_cache_size]
+ static Thread* _get_thread_cache[]; // index by [(raw_id^(raw_id>>10)) % _pd_cache_size]
static Thread* get_thread_via_cache_slowly(uintptr_t raw_id, int index);
NOT_PRODUCT(static int _tcacheHit;)
@@ -20,26 +20,26 @@
static void print_statistics() PRODUCT_RETURN;
enum Constants {
- _pd_cache_size = 128*2, // projected typical # of threads * 2
-
+ // projected typical # of threads * typical active stack pages * 2
+ _pd_cache_size = 128*2*4,
_pd_min_page_size = 4*K,
_pd_typical_stack_size = 512*K
};
static address pd_sp_address() {
- int junk;
- return (address)&junk;
+ address sp;
+ __asm__ volatile ("movl %%esp, %0" : "=r" (sp));
+ return sp;
}
static uintptr_t pd_raw_thread_id() {
- address sp = pd_sp_address();
- return (unsigned int)sp / _pd_min_page_size;
+ // _pd_min_page_size == 1 << 12
+ return ((unsigned int)pd_sp_address()) >> 12;
}
static int pd_cache_index(uintptr_t sp_page) {
- return ((sp_page / 2) /* pages tend to come in pairs */
- ^ (sp_page / (_pd_typical_stack_size/_pd_min_page_size)))
- % _pd_cache_size;
+ // _pd_cache_size == 1<<10
+ return (sp_page ^ (sp_page >> 10)) % _pd_cache_size;
}
// Java Thread
diff -ru jdk-1_5_0-scsl.b4/hotspot/src/share/vm/runtime/thread.cpp jdk-1_5_0-scsl.b4.fct2/hotspot/src/share/vm/runtime/thread.cpp
--- jdk-1_5_0-scsl.b4/hotspot/src/share/vm/runtime/thread.cpp 2007-01-24 10:40:33.000000000 +0100
+++ jdk-1_5_0-scsl.b4.fct2/hotspot/src/share/vm/runtime/thread.cpp 2007-02-22 23:53:01.000000000 +0100
@@ -104,6 +104,11 @@
delete _SR_lock;
+ // to make sure nobody finds the deleted thread as a current thread
+ // in the cache (when comparing stack pointers)
+ set_stack_base(0);
+ set_stack_size(0);
+
// clear thread local storage if the Thread is deleting itself
if (this == Thread::current()) {
ThreadLocalStorage::set_thread(NULL);
More information about the freebsd-java
mailing list