United StatesChange Country, Oracle Worldwide Web Sites Communities I am a... I want to...
Bug ID: JDK-7023931 PcDescCache::find_pc_desc should not write _last_pc_desc
JDK-7023931 : PcDescCache::find_pc_desc should not write _last_pc_desc

Details
Type:
Enhancement
Submit Date:
2011-03-02
Status:
Closed
Updated Date:
2011-04-24
Project Name:
JDK
Resolved Date:
2011-04-24
Component:
hotspot
OS:
generic
Sub-Component:
runtime
CPU:
generic
Priority:
P3
Resolution:
Fixed
Affected Versions:
hs21
Fixed Versions:
hs21 (b05)

Related Reports
Backport:

Sub Tasks

Description
A pc descriptor cache is maintained for each nmethod.  The lookup method
find_pc_desc(0 is heavily used during stack walking, and maintains the last
looked-up value in _last_pc_desc.  Unfortunately, _last_pc_desc is a shared
variable, so writes to it can cause the cache line it's in to bounce between
cores and chips, resulting in very high overhead.  On the SPECjvm2008 serial
sub-benchmark, eliminating the write results in a factor of 10 drop in last-
level-cache misses, resulting in a 10% score improvement on some platforms.

This is a useful generic improvement, as stack walking is pervasive during
jvm excecution.

                                    

Comments
SUGGESTED FIX

diff nmethod.hpp
74a75
>   PcDesc* _last_pc_desc;         // most recent pc_desc found
77c78
<   PcDescCache() { debug_only(_pc_descs[0] = NULL); }
---
>   PcDescCache() { debug_only(_last_pc_desc = NULL); }
81c82
<   PcDesc* last_pc_desc() { return _pc_descs[0]; }
---
>   PcDesc* last_pc_desc() { return _last_pc_desc; }


diff nmethod.cpp
173c173
<   int pc_desc_repeats;  // number of _pc_descs[0] hits
---
>   int pc_desc_repeats;  // number of _last_pc_desc hits
286c286
<     _pc_descs[0] = NULL; // native method; no PcDescs at all
---
>     _last_pc_desc = NULL;  // native method
292,293c292,293
<   _pc_descs[0] = initial_pc_desc + 1;  // first valid one is after sentinel
<   for (int i = 1; i < cache_size; i++)
---
>   _last_pc_desc = initial_pc_desc + 1;  // first valid one is after sentinel
>   for (int i = 0; i < cache_size; i++)
305,307c305,307
<   // Step one:  Check the most recently added value.
<   res = _pc_descs[0];
<   if (res == NULL) return NULL;  // native method; no PcDescs at all
---
>   // Step one:  Check the most recently returned value.
>   res = _last_pc_desc;
>   if (res == NULL)  return NULL;  // native method; no PcDescs at all
314c314
<   for (int i = 1; i < cache_size; i++) {
---
>   for (int i = 0; i < cache_size; i++) {
318a319
>       _last_pc_desc = res;  // record this cache hit in case of repeat
329c330
<   // Update the LRU cache by shifting pc_desc forward.
---
>   // Update the LRU cache by shifting pc_desc forward:
334a336
>   // Note:  Do not update _last_pc_desc.  It fronts for the LRU cache.
                                     
2011-03-02
EVALUATION

Ok.
                                     
2011-03-09
EVALUATION

http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/3d5a546351ef
                                     
2011-03-12
EVALUATION

http://hg.openjdk.java.net/jdk7/hotspot/hotspot/rev/3d5a546351ef
                                     
2011-03-17



Hardware and Software, Engineered to Work Together