Permalink
Browse files

Switch to a new concurrency-friendly inst var table impl.

(Description by Charles Oliver Nutter)

The new logic ensures that if two instance variable writes
occur and one or both cause the table to grow, no writes
will be lost. This was an issue before this change, and
under some circumstances early in a program's execution
(or early in a type's lifecycle) could cause data to be
lost.

See the concurrency-interest thread starting here:

http://cs.oswego.edu/pipermail/concurrency-interest/2012-December/010499.html

The implementation has three forms, for different JVM
setups:

* A version using OpenJDK 8's "fences" API for explicitly
performing memory fences.

* A version using volatile fields and Unsafe.

* A version using full synchronization.

The first version is preferred, if the JVM supports it. If
running on a non-OpenJDK 8 (b71+) JVM, the volatile +
unsafe + atomics version is used. Otherwise, we fallback
on a plain synchronized form. In most deployments, this
will not happen.

Performance of the new code does not appear to be
significantly better or worse than the old code on the
red/black benchmark we frequently use for testing.

http://i3.minus.com/ibzMT8X2FkyzJn.png

There will almost certainly be additional work in the future
to make instance variables faster, more concurrency-
friendly, and with more explicit volatility options and
guarantees.

Many thanks to @the8472 for doing the research and
development of this new logic. Thanks also to the folks
on the JVM concurrency-interest list for helping us
settle on the right amount of volatility and fencing
needed to meet our guarantees.

Squashed commit of the following:

commit 1679870
Author: the8472 <github@infinite-source.de>
Date:   Sun Jan 6 07:50:11 2013 +0100

    due to the switch from atomic field updater (which had implicit fences)
    to normal volatile access we need to upgrade to fullfence.

commit 48c3527
Author: the8472 <github@infinite-source.de>
Date:   Sun Jan 6 07:48:54 2013 +0100

    spec did not not run long enough to catch all failures -> bump up
    iterations

commit 61b8ed2
Author: Charles Oliver Nutter <headius@headius.com>
Date:   Sun Jan 6 00:11:38 2013 -0600

    Provide an Unsafe from Java 8 to build against.

commit 626a240
Author: the8472 <github@infinite-source.de>
Date:   Sun Jan 6 06:43:55 2013 +0100

    better separation of sync/unsafe code paths -> fix NPE

commit 5fdc963
Author: the8472 <github@infinite-source.de>
Date:   Sun Jan 6 05:57:42 2013 +0100

    remove stacktraces.

commit f4801ea
Author: the8472 <github@infinite-source.de>
Date:   Sun Jan 6 05:46:25 2013 +0100

    java6 has no precalculated offsets in Unsafe -> calculate them ourselves

commit d1b61e2
Author: the8472 <github@infinite-source.de>
Date:   Sat Jan 5 22:22:29 2013 +0100

    remove unused imports

commit f9b9153
Author: the8472 <github@infinite-source.de>
Date:   Sat Jan 5 21:49:32 2013 +0100

    return to original load order and java 1.6 compliance level since we
    don't need method handles in UnsafeHolder anymore

commit 93c397d
Author: the8472 <github@infinite-source.de>
Date:   Sat Jan 5 21:48:53 2013 +0100

    add java 1.8-equivalent sun.misc.Unsafe stub to build against (needs to
    be integrated into build process) and remove method handles from
    UnsafeHolder

commit ced082b
Author: the8472 <github@infinite-source.de>
Date:   Fri Jan 4 14:24:31 2013 +0100

    improve spec and bench

commit 68d51b2
Author: the8472 <github@infinite-source.de>
Date:   Fri Jan 4 14:24:05 2013 +0100

    get rid of AtomicIntegerFieldUpdater, it performs class inheritance
    checks checks. bad for opto.

commit bc865bc
Author: the8472 <github@infinite-source.de>
Date:   Fri Jan 4 14:22:57 2013 +0100

    call unsafe directly to make the profiler happy (swap for commented-out
    code to use method handles)

commit 554c6ec
Author: the8472 <github@infinite-source.de>
Date:   Fri Jan 4 04:47:53 2013 +0100

    improve test to do proper warmup

commit 0b5bee0
Author: the8472 <github@infinite-source.de>
Date:   Fri Jan 4 03:56:22 2013 +0100

    benchmark for ivar access

commit 67f4e58
Author: the8472 <github@infinite-source.de>
Date:   Fri Jan 4 00:28:35 2013 +0100

    failing test for github issue #476

commit 10db883
Author: the8472 <github@infinite-source.de>
Date:   Thu Jan 3 23:30:00 2013 +0100

    move up JDK in build order to take precedence over jsr292-mock for
    building

commit 74ac1d2
Author: the8472 <github@infinite-source.de>
Date:   Thu Jan 3 23:28:58 2013 +0100

    update to current maven-eclipse builder and project nature

commit d0e667d
Author: the8472 <github@infinite-source.de>
Date:   Thu Jan 3 23:28:09 2013 +0100

    add copyright header template and switch to java 1.7 building for now

commit 9cece18
Author: the8472 <github@infinite-source.de>
Date:   Thu Jan 3 23:26:35 2013 +0100

    initial work on stamped lock ivar table

Signed-off-by: Charles Oliver Nutter <headius@headius.com>
  • Loading branch information...
1 parent 5505eb5 commit 0e612dbd328ca94958c3db7277e4b64ffbdfc702 @the8472 the8472 committed with headius Jan 7, 2013
View
3 .classpath
@@ -35,7 +35,8 @@
<classpathentry kind="lib" path="build_lib/bcmail-jdk15-146.jar"/>
<classpathentry kind="lib" path="build_lib/bcprov-jdk15-146.jar"/>
<classpathentry kind="lib" path="build_lib/jline-2.7.jar"/>
- <classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER"/>
<classpathentry kind="lib" path="build_lib/ant.jar"/>
+ <classpathentry kind="lib" path="build_lib/unsafe-mock-1.0-SNAPSHOT.jar"/>
+ <classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER"/>
<classpathentry kind="output" path="build.eclipse"/>
</classpath>
View
4 .project
@@ -21,13 +21,13 @@
</arguments>
</buildCommand>
<buildCommand>
- <name>org.maven.ide.eclipse.maven2Builder</name>
+ <name>org.eclipse.m2e.core.maven2Builder</name>
<arguments>
</arguments>
</buildCommand>
</buildSpec>
<natures>
- <nature>org.maven.ide.eclipse.maven2Nature</nature>
+ <nature>org.eclipse.m2e.core.maven2Nature</nature>
<nature>org.eclipse.jdt.core.javanature</nature>
</natures>
<filteredResources>
View
2 .settings/org.eclipse.jdt.core.prefs
@@ -188,10 +188,8 @@ org.eclipse.jdt.core.formatter.indent_statements_compare_to_body=true
org.eclipse.jdt.core.formatter.indent_switchstatements_compare_to_cases=true
org.eclipse.jdt.core.formatter.indent_switchstatements_compare_to_switch=false
org.eclipse.jdt.core.formatter.indentation.size=4
-org.eclipse.jdt.core.formatter.insert_new_line_after_annotation=insert
org.eclipse.jdt.core.formatter.insert_new_line_after_annotation_on_field=insert
org.eclipse.jdt.core.formatter.insert_new_line_after_annotation_on_local_variable=insert
-org.eclipse.jdt.core.formatter.insert_new_line_after_annotation_on_member=insert
org.eclipse.jdt.core.formatter.insert_new_line_after_annotation_on_method=insert
org.eclipse.jdt.core.formatter.insert_new_line_after_annotation_on_package=insert
org.eclipse.jdt.core.formatter.insert_new_line_after_annotation_on_parameter=insert
View
3 .settings/org.eclipse.jdt.ui.prefs
@@ -54,13 +54,16 @@ eclipse.preferences.version=1
editor_save_participant_org.eclipse.jdt.ui.postsavelistener.cleanup=true
formatter_profile=_JRuby
formatter_settings_version=12
+org.eclipse.jdt.ui.javadoc=true
+org.eclipse.jdt.ui.text.custom_code_templates=<?xml version\="1.0" encoding\="UTF-8" standalone\="no"?><templates><template autoinsert\="true" context\="gettercomment_context" deleted\="false" description\="Comment for getter method" enabled\="true" id\="org.eclipse.jdt.ui.text.codetemplates.gettercomment" name\="gettercomment">/**\n * @return the ${bare_field_name}\n */</template><template autoinsert\="true" context\="settercomment_context" deleted\="false" description\="Comment for setter method" enabled\="true" id\="org.eclipse.jdt.ui.text.codetemplates.settercomment" name\="settercomment">/**\n * @param ${param} the ${bare_field_name} to set\n */</template><template autoinsert\="true" context\="constructorcomment_context" deleted\="false" description\="Comment for created constructors" enabled\="true" id\="org.eclipse.jdt.ui.text.codetemplates.constructorcomment" name\="constructorcomment">/**\n * ${tags}\n */</template><template autoinsert\="false" context\="filecomment_context" deleted\="false" description\="Comment for created Java files" enabled\="true" id\="org.eclipse.jdt.ui.text.codetemplates.filecomment" name\="filecomment">/*\n ***** BEGIN LICENSE BLOCK *****\n * Version\: CPL 1.0/GPL 2.0/LGPL 2.1\n *\n * The contents of this file are subject to the Common Public\n * License Version 1.0 (the "License"); you may not use this file\n * except in compliance with the License. You may obtain a copy of\n * the License at http\://www.eclipse.org/legal/cpl-v10.html\n *\n * Software distributed under the License is distributed on an "AS\n * IS" basis, WITHOUT WARRANTY OF ANY KIND, either express or\n * implied. See the License for the specific language governing\n * rights and limitations under the License.\n *\n * Alternatively, the contents of this file may be used under the terms of\n * either of the GNU General Public License Version 2 or later (the "GPL"),\n * or the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),\n * in which case the provisions of the GPL or the LGPL are applicable instead\n * of those above. If you wish to allow use of your version of this file only\n * under the terms of either the GPL or the LGPL, and not to allow others to\n * use your version of this file under the terms of the CPL, indicate your\n * decision by deleting the provisions above and replace them with the notice\n * and other provisions required by the GPL or the LGPL. If you do not delete\n * the provisions above, a recipient may use your version of this file under\n * the terms of any one of the CPL, the GPL or the LGPL.\n ***** END LICENSE BLOCK *****/</template><template autoinsert\="true" context\="typecomment_context" deleted\="false" description\="Comment for created types" enabled\="true" id\="org.eclipse.jdt.ui.text.codetemplates.typecomment" name\="typecomment">/**\n * @author ${user}\n *\n * ${tags}\n */</template><template autoinsert\="true" context\="fieldcomment_context" deleted\="false" description\="Comment for fields" enabled\="true" id\="org.eclipse.jdt.ui.text.codetemplates.fieldcomment" name\="fieldcomment">/**\n * \n */</template><template autoinsert\="true" context\="methodcomment_context" deleted\="false" description\="Comment for non-overriding methods" enabled\="true" id\="org.eclipse.jdt.ui.text.codetemplates.methodcomment" name\="methodcomment">/**\n * ${tags}\n */</template><template autoinsert\="true" context\="overridecomment_context" deleted\="false" description\="Comment for overriding methods" enabled\="true" id\="org.eclipse.jdt.ui.text.codetemplates.overridecomment" name\="overridecomment">/* (non-Javadoc)\n * ${see_to_overridden}\n */</template><template autoinsert\="true" context\="delegatecomment_context" deleted\="false" description\="Comment for delegate methods" enabled\="true" id\="org.eclipse.jdt.ui.text.codetemplates.delegatecomment" name\="delegatecomment">/**\n * ${tags}\n * ${see_to_target}\n */</template><template autoinsert\="true" context\="newtype_context" deleted\="false" description\="Newly created files" enabled\="true" id\="org.eclipse.jdt.ui.text.codetemplates.newtype" name\="newtype">${filecomment}\n${package_declaration}\n\n${typecomment}\n${type_declaration}</template><template autoinsert\="true" context\="classbody_context" deleted\="false" description\="Code in new class type bodies" enabled\="true" id\="org.eclipse.jdt.ui.text.codetemplates.classbody" name\="classbody">\n</template><template autoinsert\="true" context\="interfacebody_context" deleted\="false" description\="Code in new interface type bodies" enabled\="true" id\="org.eclipse.jdt.ui.text.codetemplates.interfacebody" name\="interfacebody">\n</template><template autoinsert\="true" context\="enumbody_context" deleted\="false" description\="Code in new enum type bodies" enabled\="true" id\="org.eclipse.jdt.ui.text.codetemplates.enumbody" name\="enumbody">\n</template><template autoinsert\="true" context\="annotationbody_context" deleted\="false" description\="Code in new annotation type bodies" enabled\="true" id\="org.eclipse.jdt.ui.text.codetemplates.annotationbody" name\="annotationbody">\n</template><template autoinsert\="true" context\="catchblock_context" deleted\="false" description\="Code in new catch blocks" enabled\="true" id\="org.eclipse.jdt.ui.text.codetemplates.catchblock" name\="catchblock">// ${todo} Auto-generated catch block\n${exception_var}.printStackTrace();</template><template autoinsert\="true" context\="methodbody_context" deleted\="false" description\="Code in created method stubs" enabled\="true" id\="org.eclipse.jdt.ui.text.codetemplates.methodbody" name\="methodbody">// ${todo} Auto-generated method stub\n${body_statement}</template><template autoinsert\="true" context\="constructorbody_context" deleted\="false" description\="Code in created constructor stubs" enabled\="true" id\="org.eclipse.jdt.ui.text.codetemplates.constructorbody" name\="constructorbody">${body_statement}\n// ${todo} Auto-generated constructor stub</template><template autoinsert\="true" context\="getterbody_context" deleted\="false" description\="Code in created getters" enabled\="true" id\="org.eclipse.jdt.ui.text.codetemplates.getterbody" name\="getterbody">return ${field};</template><template autoinsert\="true" context\="setterbody_context" deleted\="false" description\="Code in created setters" enabled\="true" id\="org.eclipse.jdt.ui.text.codetemplates.setterbody" name\="setterbody">${field} \= ${param};</template></templates>
sp_cleanup.add_default_serial_version_id=true
sp_cleanup.add_generated_serial_version_id=false
sp_cleanup.add_missing_annotations=true
sp_cleanup.add_missing_deprecated_annotations=true
sp_cleanup.add_missing_methods=false
sp_cleanup.add_missing_nls_tags=false
sp_cleanup.add_missing_override_annotations=true
+sp_cleanup.add_missing_override_annotations_interface_methods=false
sp_cleanup.add_serial_version_id=false
sp_cleanup.always_use_blocks=true
sp_cleanup.always_use_parentheses_in_expressions=false
View
79 bench/core/basic_object/ivar_access_bench.rb
@@ -0,0 +1,79 @@
+require 'benchmark'
+
+
+SIMPLE_TIMES = 10_000_000
+GROWTH_TIMES = 8_000
+
+# prepare dummy objects
+default_object = Class.new.new
+default_object.instance_variable_set(:@foo,2)
+
+disposable_objects = (0..10).map{Class.new.new}
+
+simple_reader = proc do |o|
+ result = nil;
+ for i in (0..SIMPLE_TIMES)
+ result = o.instance_variable_get(:@foo)
+ result = o.instance_variable_get(:@foo)
+ result = o.instance_variable_get(:@foo)
+ result = o.instance_variable_get(:@foo)
+ result = o.instance_variable_get(:@foo)
+ result = o.instance_variable_get(:@foo)
+ result = o.instance_variable_get(:@foo)
+ result = o.instance_variable_get(:@foo)
+ result = o.instance_variable_get(:@foo)
+ result = o.instance_variable_get(:@foo)
+ result = o.instance_variable_get(:@foo)
+ end
+ result
+end
+
+simple_writer = proc do |o|
+ for i in (0..SIMPLE_TIMES)
+ o.instance_variable_set(:@foo,0)
+ o.instance_variable_set(:@foo,1)
+ o.instance_variable_set(:@foo,2)
+ o.instance_variable_set(:@foo,3)
+ o.instance_variable_set(:@foo,4)
+ o.instance_variable_set(:@foo,5)
+ o.instance_variable_set(:@foo,6)
+ o.instance_variable_set(:@foo,7)
+ o.instance_variable_set(:@foo,8)
+ o.instance_variable_set(:@foo,9)
+ end
+end
+
+Benchmark.bmbm do |b|
+ b.report "baseline x#{SIMPLE_TIMES}" do
+ for i in (0..SIMPLE_TIMES)
+ 1
+ end
+ end
+
+ b.report "single threaded reads x#{SIMPLE_TIMES}" do
+ simple_reader.call(default_object)
+ end
+
+ b.report "single threaded writes x#{SIMPLE_TIMES}" do
+ simple_writer.call(default_object)
+ end
+
+ b.report "two reader threads x#{SIMPLE_TIMES}" do
+ t1 = Thread.new{simple_reader.call(default_object)}
+ t2 = Thread.new{simple_reader.call(default_object)}
+ t1.join;t2.join
+ end
+
+ b.report "one reader, one writer x#{SIMPLE_TIMES}" do
+ t1 = Thread.new{simple_reader.call(default_object)}
+ t2 = Thread.new{simple_writer.call(default_object)}
+ t1.join;t2.join
+ end
+
+ b.report "single threaded growth x#{GROWTH_TIMES}" do
+ o = disposable_objects.pop
+ (0..GROWTH_TIMES).each{|i| o.instance_variable_set(:"@bar_#{i}",1)}
+ end
+
+
+end
View
1 build.xml
@@ -291,6 +291,7 @@
<compilerarg line="-XDignore.symbol.file=true"/>
<compilerarg line="-J-Dfile.encoding=UTF-8"/>
<compilerarg line="-J-Duser.language=en"/>
+ <compilerarg line="-J-Xbootclasspath/p:build_lib/unsafe-mock-1.0-SNAPSHOT.jar"/>
<compilerarg line="-processor org.jruby.anno.AnnotationBinder"/>
</javac>
View
190 build_lib/sun/misc/Unsafe.java
@@ -0,0 +1,190 @@
+/*
+ * **** BEGIN LICENSE BLOCK ***** Version: CPL 1.0/GPL 2.0/LGPL 2.1 The contents
+ * of this file are subject to the Common Public License Version 1.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * http://www.eclipse.org/legal/cpl-v10.html Software distributed under the
+ * License is distributed on an "AS IS" basis, WITHOUT WARRANTY OF ANY KIND,
+ * either express or implied. See the License for the specific language
+ * governing rights and limitations under the License. Alternatively, the
+ * contents of this file may be used under the terms of either of the GNU
+ * General Public License Version 2 or later (the "GPL"), or the GNU Lesser
+ * General Public License Version 2.1 or later (the "LGPL"), in which case the
+ * provisions of the GPL or the LGPL are applicable instead of those above. If
+ * you wish to allow use of your version of this file only under the terms of
+ * either the GPL or the LGPL, and not to allow others to use your version of
+ * this file under the terms of the CPL, indicate your decision by deleting the
+ * provisions above and replace them with the notice and other provisions
+ * required by the GPL or the LGPL. If you do not delete the provisions above, a
+ * recipient may use your version of this file under the terms of any one of the
+ * CPL, the GPL or the LGPL.**** END LICENSE BLOCK ****
+ */
+package sun.misc;
+
+import java.lang.reflect.Field;
+import java.security.ProtectionDomain;
+
+public abstract class Unsafe {
+
+ private static Unsafe theUnsafe;
+ public abstract int getInt(Object o, long offset);
+ public abstract void putInt(Object o, long offset, int x);
+ public abstract Object getObject(Object o, long offset);
+ public abstract void putObject(Object o, long offset, Object x);
+ public abstract boolean getBoolean(Object o, long offset);
+ public abstract void putBoolean(Object o, long offset, boolean x);
+ public abstract byte getByte(Object o, long offset);
+ public abstract void putByte(Object o, long offset, byte x);
+ public abstract short getShort(Object o, long offset);
+ public abstract void putShort(Object o, long offset, short x);
+ public abstract char getChar(Object o, long offset);
+ public abstract void putChar(Object o, long offset, char x);
+ public abstract long getLong(Object o, long offset);
+ public abstract void putLong(Object o, long offset, long x);
+ public abstract float getFloat(Object o, long offset);
+ public abstract void putFloat(Object o, long offset, float x);
+ public abstract double getDouble(Object o, long offset);
+ public abstract void putDouble(Object o, long offset, double x);
+ @Deprecated
+ public abstract int getInt(Object o, int offset);
+ @Deprecated
+ public abstract void putInt(Object o, int offset, int x);
+ @Deprecated
+ public abstract Object getObject(Object o, int offset);
+ @Deprecated
+ public abstract void putObject(Object o, int offset, Object x);
+ @Deprecated
+ public abstract boolean getBoolean(Object o, int offset);
+ @Deprecated
+ public abstract void putBoolean(Object o, int offset, boolean x);
+ @Deprecated
+ public abstract byte getByte(Object o, int offset);
+ @Deprecated
+ public abstract void putByte(Object o, int offset, byte x);
+ @Deprecated
+ public abstract short getShort(Object o, int offset);
+ @Deprecated
+ public abstract void putShort(Object o, int offset, short x);
+ @Deprecated
+ public abstract char getChar(Object o, int offset);
+ @Deprecated
+ public abstract void putChar(Object o, int offset, char x);
+ @Deprecated
+ public abstract long getLong(Object o, int offset);
+ @Deprecated
+ public abstract void putLong(Object o, int offset, long x);
+ @Deprecated
+ public abstract float getFloat(Object o, int offset);
+ @Deprecated
+ public abstract void putFloat(Object o, int offset, float x);
+ @Deprecated
+ public abstract double getDouble(Object o, int offset);
+ @Deprecated
+ public abstract void putDouble(Object o, int offset, double x);
+ public abstract byte getByte(long address);
+ public abstract void putByte(long address, byte x);
+ public abstract short getShort(long address);
+ public abstract void putShort(long address, short x);
+ public abstract char getChar(long address);
+ public abstract void putChar(long address, char x);
+ public abstract int getInt(long address);
+ public abstract void putInt(long address, int x);
+ public abstract long getLong(long address);
+ public abstract void putLong(long address, long x);
+ public abstract float getFloat(long address);
+ public abstract void putFloat(long address, float x);
+ public abstract double getDouble(long address);
+ public abstract void putDouble(long address, double x);
+ public abstract long getAddress(long address);
+ public abstract void putAddress(long address, long x);
+ public abstract long allocateMemory(long bytes);
+ public abstract long reallocateMemory(long address, long bytes);
+ public abstract void setMemory(Object o, long offset, long bytes, byte value);
+ public abstract void setMemory(long address, long bytes, byte value);
+ public abstract void copyMemory(Object srcBase, long srcOffset, Object destBase,
+ long destOffset, long bytes);
+ public abstract void copyMemory(long srcAddress, long destAddress, long bytes);
+
+ public abstract void freeMemory(long address);
+ public static final int INVALID_FIELD_OFFSET = 0;
+ @Deprecated
+ public abstract int fieldOffset(Field f);
+ @Deprecated
+ public abstract Object staticFieldBase(Class<?> c);
+ public abstract long staticFieldOffset(Field f);
+ public abstract long objectFieldOffset(Field f);
+ public abstract Object staticFieldBase(Field f);
+ public abstract boolean shouldBeInitialized(Class<?> c);
+ public abstract void ensureClassInitialized(Class<?> c);
+ public abstract int arrayBaseOffset(Class<?> arrayClass);
+
+ public static final int ARRAY_BOOLEAN_BASE_OFFSET = 0;
+ public static final int ARRAY_BYTE_BASE_OFFSET = 0;
+ public static final int ARRAY_SHORT_BASE_OFFSET = 0;
+ public static final int ARRAY_CHAR_BASE_OFFSET = 0;
+ public static final int ARRAY_INT_BASE_OFFSET = 0;
+ public static final int ARRAY_LONG_BASE_OFFSET = 0;
+ public static final int ARRAY_FLOAT_BASE_OFFSET = 0;
+ public static final int ARRAY_DOUBLE_BASE_OFFSET = 0;
+ public static final int ARRAY_OBJECT_BASE_OFFSET = 0;
+ public abstract int arrayIndexScale(Class<?> arrayClass);
+
+ public static final int ARRAY_BOOLEAN_INDEX_SCALE = 0;
+ public static final int ARRAY_BYTE_INDEX_SCALE = 0;
+ public static final int ARRAY_SHORT_INDEX_SCALE = 0;
+ public static final int ARRAY_CHAR_INDEX_SCALE = 0;
+ public static final int ARRAY_INT_INDEX_SCALE = 0;
+ public static final int ARRAY_LONG_INDEX_SCALE = 0;
+ public static final int ARRAY_FLOAT_INDEX_SCALE = 0;
+ public static final int ARRAY_DOUBLE_INDEX_SCALE = 0;
+ public static final int ARRAY_OBJECT_INDEX_SCALE = 0;
+ public abstract int addressSize();
+ public static final int ADDRESS_SIZE = theUnsafe.addressSize();
+ public abstract int pageSize();
+ public abstract Class<?> defineClass(String name, byte[] b, int off, int len,
+ ClassLoader loader, ProtectionDomain protectionDomain);
+ public abstract Class<?> defineClass(String name, byte[] b, int off, int len);
+ public abstract Class<?> defineAnonymousClass(Class<?> hostClass, byte[] data,
+ Object[] cpPatches);
+ public abstract Object allocateInstance(Class<?> cls) throws InstantiationException;
+ public abstract void monitorEnter(Object o);
+ public abstract void monitorExit(Object o);
+ public abstract boolean tryMonitorEnter(Object o);
+ public abstract void throwException(Throwable ee);
+ public abstract boolean compareAndSwapObject(Object o, long offset, Object expected, Object x);
+ public abstract boolean compareAndSwapInt(Object o, long offset, int expected, int x);
+ public abstract boolean compareAndSwapLong(Object o, long offset, long expected, long x);
+ public abstract Object getObjectVolatile(Object o, long offset);
+ public abstract void putObjectVolatile(Object o, long offset, Object x);
+ public abstract int getIntVolatile(Object o, long offset);
+ public abstract void putIntVolatile(Object o, long offset, int x);
+ public abstract boolean getBooleanVolatile(Object o, long offset);
+ public abstract void putBooleanVolatile(Object o, long offset, boolean x);
+ public abstract byte getByteVolatile(Object o, long offset);
+ public abstract void putByteVolatile(Object o, long offset, byte x);
+ public abstract short getShortVolatile(Object o, long offset);
+ public abstract void putShortVolatile(Object o, long offset, short x);
+ public abstract char getCharVolatile(Object o, long offset);
+ public abstract void putCharVolatile(Object o, long offset, char x);
+ public abstract long getLongVolatile(Object o, long offset);
+ public abstract void putLongVolatile(Object o, long offset, long x);
+ public abstract float getFloatVolatile(Object o, long offset);
+ public abstract void putFloatVolatile(Object o, long offset, float x);
+ public abstract double getDoubleVolatile(Object o, long offset);
+ public abstract void putDoubleVolatile(Object o, long offset, double x);
+ public abstract void putOrderedObject(Object o, long offset, Object x);
+ public abstract void putOrderedInt(Object o, long offset, int x);
+ public abstract void putOrderedLong(Object o, long offset, long x);
+ public abstract void unpark(Object thread);
+ public abstract void park(boolean isAbsolute, long time);
+ public abstract int getLoadAverage(double[] loadavg, int nelems);
+ public abstract int getAndAddInt(Object o, long offset, int delta);
+ public abstract long getAndAddLong(Object o, long offset, long delta);
+ public abstract int getAndSetInt(Object o, long offset, int newValue);
+ public abstract long getAndSetLong(Object o, long offset, long newValue);
+ public abstract Object getAndSetObject(Object o, long offset, Object newValue);
+ public abstract void loadFence();
+ public abstract void storeFence();
+ public abstract void fullFence();
+
+}
View
BIN build_lib/unsafe-mock-1.0-SNAPSHOT.jar
Binary file not shown.
View
2 nbproject/project.xml
@@ -248,7 +248,7 @@
<java-data xmlns="http://www.netbeans.org/ns/freeform-project-java/3">
<compilation-unit>
<package-root>${src.dir}</package-root>
- <classpath mode="compile">build_lib/junit.jar:build_lib/jna.jar:build_lib/nailgun-0.7.1.jar:build_lib/joni.jar:build_lib/dynalang-0.3.jar:build_lib/invokedynamic.jar:build_lib/jcodings.jar:build_lib/bytelist.jar:build_lib/jffi.jar:build_lib/yydebug.jar:build_lib/bsf.jar:build_lib/jnr-ffi.jar:build_lib/jsr292-mock.jar:build_lib/jgrapht-jdk1.5.jar:build_lib/jnr-netdb.jar:build_lib/jnr-posix.jar:build_lib/livetribe-jsr223-2.0.6.jar:build_lib/ant.jar:build_lib/org.osgi.core-4.2.0.jar:build_lib/jnr-constants.jar:build_lib/slf4j-api-1.6.1.jar:build_lib/asm-4.0.jar:build_lib/asm-analysis-4.0.jar:build_lib/asm-commons-4.0.jar:build_lib/asm-tree-4.0.jar:build_lib/asm-util-4.0.jar:build_lib/jzlib-gzip.jar:build_lib/coro-mock-1.0-SNAPSHOT.jar:build_lib/jzlib-1.1.0.jar:build_lib/joda-time-2.1.jar:build_lib/snakeyaml-1.11.jar:build_lib/invokebinder.jar:build_lib/jnr-unixsocket.jar:build_lib/jline-2.7.jar:build_lib/bcmail-jdk15-146.jar:build_lib/bcprov-jdk15-146.jar:build_lib/jnr-enxio.jar</classpath>
+ <classpath mode="compile">build_lib/junit.jar:build_lib/jna.jar:build_lib/nailgun-0.7.1.jar:build_lib/joni.jar:build_lib/dynalang-0.3.jar:build_lib/invokedynamic.jar:build_lib/jcodings.jar:build_lib/bytelist.jar:build_lib/jffi.jar:build_lib/yydebug.jar:build_lib/bsf.jar:build_lib/jnr-ffi.jar:build_lib/jsr292-mock.jar:build_lib/jgrapht-jdk1.5.jar:build_lib/jnr-netdb.jar:build_lib/jnr-posix.jar:build_lib/livetribe-jsr223-2.0.6.jar:build_lib/ant.jar:build_lib/org.osgi.core-4.2.0.jar:build_lib/jnr-constants.jar:build_lib/slf4j-api-1.6.1.jar:build_lib/asm-4.0.jar:build_lib/asm-analysis-4.0.jar:build_lib/asm-commons-4.0.jar:build_lib/asm-tree-4.0.jar:build_lib/asm-util-4.0.jar:build_lib/jzlib-gzip.jar:build_lib/coro-mock-1.0-SNAPSHOT.jar:build_lib/jzlib-1.1.0.jar:build_lib/joda-time-2.1.jar:build_lib/snakeyaml-1.11.jar:build_lib/invokebinder.jar:build_lib/jnr-unixsocket.jar:build_lib/jline-2.7.jar:build_lib/bcmail-jdk15-146.jar:build_lib/bcprov-jdk15-146.jar:build_lib/jnr-enxio.jar:build_lib/unsafe-mock-1.0-SNAPSHOT.jar</classpath>
<built-to>${jruby.classes.dir}</built-to>
<built-to>${lib.dir}/jruby.jar</built-to>
<javadoc-built-to>docs/api</javadoc-built-to>
View
34 spec/regression/GH-476_ivar_concurrency_spec.rb
@@ -0,0 +1,34 @@
+require 'rspec'
+
+describe 'Accessing instance variables' do
+ it 'should not lose concurrent writes under growth operations' do
+ lost_writes = false
+
+ (0..1000).each do |i|
+ clazz = Class.new
+ object = clazz.new
+
+ mutate = true
+
+ # probing thread
+ t1 = Thread.new do
+ (0..10000).each do |i|
+ object.instance_variable_set(:@foo, i)
+ lost_writes = true if object.instance_variable_get(:@foo) != i
+ end
+
+ end
+
+ # mutating thread
+ t2 = Thread.new{(0..10000).each{break unless mutate;object.instance_variable_set(:"@bar_#{rand(100000)}",1)}}
+
+ t1.join
+ mutate = false
+ t2.join
+
+ break if lost_writes
+ end
+
+ lost_writes.should be_false
+ end
+end
View
241 src/org/jruby/RubyBasicObject.java
@@ -31,14 +31,12 @@
import java.io.ObjectInputStream;
import java.io.ObjectOutputStream;
import java.io.Serializable;
-import java.security.AccessControlException;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean;
-import java.util.concurrent.atomic.AtomicReferenceFieldUpdater;
import org.jruby.anno.JRubyMethod;
import org.jruby.common.IRubyWarnings.ID;
@@ -67,6 +65,7 @@
import org.jruby.util.TypeConverter;
import org.jruby.util.log.Logger;
import org.jruby.util.log.LoggerFactory;
+import org.jruby.util.unsafe.UnsafeHolder;
import static org.jruby.javasupport.util.RuntimeHelpers.invokedynamic;
import static org.jruby.runtime.invokedynamic.MethodNames.OP_EQUAL;
@@ -114,7 +113,13 @@
protected int flags;
// variable table, lazily allocated as needed (if needed)
- private transient volatile Object[] varTable;
+ private transient Object[] varTable;
+
+ // locking stamp for Unsafe ops updating the vartable
+ private transient volatile int varTableStamp;
+
+ private static final long VAR_TABLE_OFFSET = UnsafeHolder.fieldOffset(RubyBasicObject.class, "varTable");
+ private static final long STAMP_OFFSET = UnsafeHolder.fieldOffset(RubyBasicObject.class, "varTableStamp");
/**
* The error message used when some one tries to modify an
@@ -1186,110 +1191,93 @@ public void removeFinalizers() {
return varTable;
}
- private static final AtomicReferenceFieldUpdater VARTABLE_UPDATER;
-
- static {
- AtomicReferenceFieldUpdater updater = null;
- try {
- updater = AtomicReferenceFieldUpdater.newUpdater(RubyBasicObject.class, Object[].class, "varTable");
- } catch (RuntimeException re) {
- if (re.getCause() instanceof AccessControlException) {
- // security prevented creation; fall back on synchronized assignment
- } else {
- throw re;
- }
- }
- VARTABLE_UPDATER = updater;
- }
-
-
- /**
- * Get variable table for write purposes. Initializes if uninitialized, and
- * resizes if necessary.
- */
- protected final Object[] getVariableTableForWrite(int index) {
- if (VARTABLE_UPDATER == null) {
- return getVariableTableForWriteSynchronized(index);
- } else {
- return getVariableTableForWriteAtomic(index);
- }
- }
-
- /**
- * Get the variable table for write. If it is not set or not of the right size,
- * synchronize against the object and prepare it accordingly.
- *
- * @param index the index of the value soon to be set
- * @return the var table, ready for setting
- */
- private Object[] getVariableTableForWriteSynchronized(int index) {
- Object[] myVarTable = varTable;
- if (myVarTable == null || myVarTable.length <= index) {
- synchronized (this) {
- myVarTable = varTable;
-
- if (myVarTable == null) {
- return varTable = new Object[getMetaClass().getRealClass().getVariableTableSizeWithExtras()];
- } else if (myVarTable.length <= index) {
- Object[] newTable = new Object[getMetaClass().getRealClass().getVariableTableSizeWithExtras()];
- System.arraycopy(myVarTable, 0, newTable, 0, myVarTable.length);
- return varTable = newTable;
- } else {
- return myVarTable;
- }
- }
- }
-
- return varTable;
- }
-
-
- /**
- * Get the variable table for write. If it is not set or not of the right size,
- * atomically update it with an appropriate value.
- *
- * @param index the index of the value soon to be set
- * @return the var table, ready for setting
- */
- private Object[] getVariableTableForWriteAtomic(int index) {
- while (true) {
- Object[] myVarTable = varTable;
- Object[] newTable;
-
- if (myVarTable == null) {
- newTable = new Object[metaClass.getRealClass().getVariableTableSizeWithExtras()];
- } else if (myVarTable.length <= index) {
- newTable = new Object[metaClass.getRealClass().getVariableTableSizeWithExtras()];
- System.arraycopy(myVarTable, 0, newTable, 0, myVarTable.length);
- } else {
- return myVarTable;
- }
-
- // proceed with atomic update of table, or retry
- if (VARTABLE_UPDATER.compareAndSet(this, myVarTable, newTable)) {
- return newTable;
- }
- }
- }
-
public Object getVariable(int index) {
Object[] ivarTable;
if (index < 0 || (ivarTable = getVariableTableForRead()) == null) return null;
if (ivarTable.length > index) return ivarTable[index];
return null;
}
-
+
public void setVariable(int index, Object value) {
ensureInstanceVariablesSettable();
if (index < 0) return;
- Object[] ivarTable = getVariableTableForWrite(index);
- ivarTable[index] = value;
+ setVariableInternal(index, value);
+ }
+
+ protected final void setVariableInternal(int index, Object value) {
+ if(UnsafeHolder.U == null)
+ setVariableSynchronized(index,value);
+ else
+ setVariableStamped(index,value);
+ }
+
+ private void setVariableSynchronized(int index, Object value) {
+ synchronized (this) {
+ Object[] currentTable = varTable;
+
+ if (currentTable == null) {
+ varTable = currentTable = new Object[getMetaClass().getRealClass().getVariableTableSizeWithExtras()];
+ } else if (currentTable.length <= index) {
+ Object[] newTable = new Object[getMetaClass().getRealClass().getVariableTableSizeWithExtras()];
+ System.arraycopy(currentTable, 0, newTable, 0, currentTable.length);
+ varTable = newTable;
+ }
+
+ varTable[index] = value;
+ }
+ }
+
+ private void setVariableStamped(int index, Object value) {
+
+ for(;;) {
+ int currentStamp = varTableStamp;
+ // spin-wait if odd
+ if((currentStamp & 0x01) != 0)
+ continue;
+
+ Object[] currentTable = (Object[]) UnsafeHolder.U.getObjectVolatile(this, VAR_TABLE_OFFSET);
+
+ if(currentTable == null || index >= currentTable.length)
+ {
+ // try to acquire exclusive access to the varTable field
+ if(!UnsafeHolder.U.compareAndSwapInt(this, STAMP_OFFSET, currentStamp, ++currentStamp))
+ continue;
+
+ Object[] newTable = new Object[getMetaClass().getRealClass().getVariableTableSizeWithExtras()];
+ if(currentTable != null)
+ System.arraycopy(currentTable, 0, newTable, 0, currentTable.length);
+ newTable[index] = value;
+ UnsafeHolder.U.putOrderedObject(this, VAR_TABLE_OFFSET, newTable);
+
+ // release exclusive access
+ varTableStamp = currentStamp + 1;
+ } else {
+ // shared access to varTable field.
+
+ if(UnsafeHolder.SUPPORTS_FENCES) {
+ currentTable[index] = value;
+ UnsafeHolder.fullFence();
+ } else {
+ // TODO: maybe optimize by read and checking current value before setting
+ UnsafeHolder.U.putObjectVolatile(currentTable, UnsafeHolder.ARRAY_OBJECT_BASE_OFFSET + UnsafeHolder.ARRAY_OBJECT_INDEX_SCALE * index, value);
+ }
+
+ // validate stamp. redo on concurrent modification
+ if(varTableStamp != currentStamp)
+ continue;
+
+ }
+
+ break;
+ }
+
+
}
+
private void setObjectId(int index, long value) {
if (index < 0) return;
- Object[] ivarTable = getVariableTableForWrite(index);
- ivarTable[index] = value;
+ setVariableInternal(index, value);
}
public final Object getNativeHandle() {
@@ -1298,8 +1286,7 @@ public final Object getNativeHandle() {
public final void setNativeHandle(Object value) {
int index = getMetaClass().getRealClass().getNativeHandleAccessorField().getVariableAccessorForWrite().getIndex();
- Object[] ivarTable = getVariableTableForWrite(index);
- ivarTable[index] = value;
+ setVariableInternal(index, value);
}
public final Object getFFIHandle() {
@@ -1308,8 +1295,7 @@ public final Object getFFIHandle() {
public final void setFFIHandle(Object value) {
int index = getMetaClass().getRealClass().getFFIHandleAccessorField().getVariableAccessorForWrite().getIndex();
- Object[] ivarTable = getVariableTableForWrite(index);
- ivarTable[index] = value;
+ setVariableInternal(index, value);
}
//
@@ -1460,7 +1446,7 @@ public Object removeInternalVariable(String name) {
assert !IdUtil.isRubyVariable(name);
return variableTableRemove(name);
}
-
+
/**
* Sync one this object's variables with other's - this is used to make
* rbClone work correctly.
@@ -1471,17 +1457,39 @@ public void syncVariables(IRubyObject other) {
boolean sameTable = otherRealClass == realClass;
if (sameTable) {
- RubyClass.VariableAccessor objIdAccessor = otherRealClass.getObjectIdAccessorField().getVariableAccessorForRead();
+ int idIndex = otherRealClass.getObjectIdAccessorField().getVariableAccessorForRead().getIndex();
+
+
Object[] otherVars = ((RubyBasicObject) other).varTable;
- int otherLength = otherVars.length;
- Object[] myVars = getVariableTableForWrite(otherLength - 1);
- System.arraycopy(otherVars, 0, myVars, 0, otherLength);
-
- // null out object ID so we don't share it
- int objIdIndex = objIdAccessor.getIndex();
- if (objIdIndex > 0 && objIdIndex < myVars.length) {
- myVars[objIdIndex] = null;
+
+ if(UnsafeHolder.U == null)
+ {
+ synchronized (this) {
+ varTable = makeSyncedTable(varTable, otherVars, idIndex);
+ }
+ } else {
+ for(;;) {
+ int oldStamp = varTableStamp;
+ // wait for read mode
+ if((oldStamp & 0x01) == 1)
+ continue;
+ // acquire exclusive write mode
+ if(!UnsafeHolder.U.compareAndSwapInt(this, STAMP_OFFSET, oldStamp, ++oldStamp))
+ continue;
+
+ Object[] currentTable = (Object[]) UnsafeHolder.U.getObjectVolatile(this, VAR_TABLE_OFFSET);
+ Object[] newTable = makeSyncedTable(currentTable,otherVars, idIndex);
+
+ UnsafeHolder.U.putOrderedObject(this, VAR_TABLE_OFFSET, newTable);
+
+ // release write mode
+ varTableStamp = oldStamp+1;
+ break;
+ }
+
+
}
+
} else {
for (Map.Entry<String, RubyClass.VariableAccessor> entry : otherRealClass.getVariableAccessorsForRead().entrySet()) {
RubyClass.VariableAccessor accessor = entry.getValue();
@@ -1498,7 +1506,20 @@ public void syncVariables(IRubyObject other) {
}
}
-
+ private static Object[] makeSyncedTable(Object[] currentTable, Object[] otherTable, int objectIdIdx) {
+ if(currentTable == null || currentTable.length < otherTable.length)
+ currentTable = otherTable.clone();
+ else
+ System.arraycopy(otherTable, 0, currentTable, 0, otherTable.length);
+
+ // null out object ID so we don't share it
+ if (objectIdIdx > 0 && objectIdIdx < currentTable.length) {
+ currentTable[objectIdIdx] = null;
+ }
+
+ return currentTable;
+ }
+
//
// INSTANCE VARIABLE API METHODS
//
@@ -2883,7 +2904,7 @@ public IRubyObject instance_variable_get(ThreadContext context, IRubyObject name
* fred.inspect #=> "#<Fred:0x401b3da8 @a=\"dog\", @b=99, @c=\"cat\">"
*/
public IRubyObject instance_variable_set(IRubyObject name, IRubyObject value) {
- ensureInstanceVariablesSettable();
+ // no need to check for ensureInstanceVariablesSettable() here, that'll happen downstream in setVariable
return (IRubyObject)variableTableStore(validateInstanceVariable(name.asJavaString()), value);
}
View
105 src/org/jruby/util/unsafe/UnsafeHolder.java
@@ -0,0 +1,105 @@
+/*
+ ***** BEGIN LICENSE BLOCK *****
+ * Version: CPL 1.0/GPL 2.0/LGPL 2.1
+ *
+ * The contents of this file are subject to the Common Public
+ * License Version 1.0 (the "License"); you may not use this file
+ * except in compliance with the License. You may obtain a copy of
+ * the License at http://www.eclipse.org/legal/cpl-v10.html
+ *
+ * Software distributed under the License is distributed on an "AS
+ * IS" basis, WITHOUT WARRANTY OF ANY KIND, either express or
+ * implied. See the License for the specific language governing
+ * rights and limitations under the License.
+ *
+ * Alternatively, the contents of this file may be used under the terms of
+ * either of the GNU General Public License Version 2 or later (the "GPL"),
+ * or the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
+ * in which case the provisions of the GPL or the LGPL are applicable instead
+ * of those above. If you wish to allow use of your version of this file only
+ * under the terms of either the GPL or the LGPL, and not to allow others to
+ * use your version of this file under the terms of the CPL, indicate your
+ * decision by deleting the provisions above and replace them with the notice
+ * and other provisions required by the GPL or the LGPL. If you do not delete
+ * the provisions above, a recipient may use your version of this file under
+ * the terms of any one of the CPL, the GPL or the LGPL.
+ ***** END LICENSE BLOCK *****/
+package org.jruby.util.unsafe;
+
+import java.lang.reflect.Field;
+import java.lang.reflect.Method;
+
+public final class UnsafeHolder {
+
+ private UnsafeHolder(){}
+
+ /**
+ * Holds a reference to Unsafe if available, null otherwise.
+ */
+ public static final sun.misc.Unsafe U = loadUnsafe();
+
+ public static final boolean SUPPORTS_FENCES = supportsFences();
+ public static final long ARRAY_OBJECT_BASE_OFFSET = arrayObjectBaseOffset();
+ public static final long ARRAY_OBJECT_INDEX_SCALE = arrayObjectIndexScale();
+
+ private static sun.misc.Unsafe loadUnsafe() {
+ try {
+ Class unsafeClass = Class.forName("sun.misc.Unsafe");
+ Field f = unsafeClass.getDeclaredField("theUnsafe");
+ f.setAccessible(true);
+ return (sun.misc.Unsafe) f.get(null);
+ } catch (Exception e) {
+ return null;
+ }
+ }
+
+ private static long arrayObjectBaseOffset() {
+ if(U == null)
+ return 0;
+ return U.arrayBaseOffset(Object[].class);
+ }
+
+ private static long arrayObjectIndexScale() {
+ if(U == null)
+ return 0;
+ return U.arrayIndexScale(Object[].class);
+ }
+
+ private static boolean supportsFences() {
+ if(U == null)
+ return false;
+ try {
+ Method m = U.getClass().getDeclaredMethod("fullFence");
+ if(m != null)
+ return true;
+ } catch (Exception e) {
+ }
+ return false;
+ }
+
+ public static long fieldOffset(Class clazz, String name) {
+ if(U == null)
+ return -1;
+ try {
+ return U.objectFieldOffset(clazz.getDeclaredField(name));
+ } catch (Exception e) {
+ return sun.misc.Unsafe.INVALID_FIELD_OFFSET;
+ }
+ }
+
+ //// The following methods are Java8 only. They will throw undefined method errors if invoked without checking for fence support
+
+ public static void fullFence() {
+ U.fullFence();
+ }
+
+ public static void loadFence() {
+ U.loadFence();
+ }
+
+ public static void storeFence() {
+ U.storeFence();
+ }
+
+
+}

0 comments on commit 0e612db

Please sign in to comment.