Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: jruby/jruby
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: f7773002336c
Choose a base ref
...
head repository: jruby/jruby
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: 0820935d0cfe
Choose a head ref
  • 3 commits
  • 4 files changed
  • 1 contributor

Commits on Dec 31, 2014

  1. Reduce complexity of symbol table and clean up dead references.

    The naive approach required a bit more finesse.
    
    * Eliminated ByteList => RubySymbol table and added ByteList to
      SymbolEntry.
    * Modified cache-miss lookup logic clean out dead references in
      same bucket.
    * Other reformatting and cleanup.
    headius committed Dec 31, 2014
    Copy the full SHA
    96d5862 View commit details
  2. Copy the full SHA
    f1f4bb1 View commit details
  3. 2
    Copy the full SHA
    0820935 View commit details
7 changes: 7 additions & 0 deletions core/src/main/java/org/jruby/Ruby.java
Original file line number Diff line number Diff line change
@@ -42,6 +42,7 @@
import org.jruby.compiler.Constantizable;
import org.jruby.compiler.NotCompilableException;
import org.jruby.ext.jruby.JRubyLibrary;
import org.jruby.ext.thread.ThreadLibrary;
import org.jruby.ir.IRScriptBody;
import org.jruby.parser.StaticScope;
import org.objectweb.asm.util.TraceClassVisitor;
@@ -1535,6 +1536,9 @@ private void initCore() {
RubyEnumerator.defineEnumerator(this);
}

// Fiber depends on thread library, so we load it here
new ThreadLibrary().load(this, false);

new ThreadFiberLibrary().load(this, false);

TracePoint.createTracePointClass(this);
@@ -1727,6 +1731,9 @@ private void initBuiltins() {
loadService.provide("rational.jar");
loadService.provide("complex.jar");

// we define the classes at boot because we need them
addBuiltinIfAllowed("thread.rb", Library.DUMMY);

if(RubyInstanceConfig.NATIVE_NET_PROTOCOL) {
addLazyBuiltin("net/protocol.rb", "net/protocol", "org.jruby.ext.net.protocol.NetProtocolBufferedIOLibrary");
}
269 changes: 147 additions & 122 deletions core/src/main/java/org/jruby/RubySymbol.java
Original file line number Diff line number Diff line change
@@ -63,7 +63,6 @@
import org.jruby.util.SipHashInline;

import java.lang.ref.WeakReference;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.locks.ReentrantLock;

import static org.jruby.util.StringSupport.codeLength;
@@ -636,13 +635,12 @@ public static ByteList symbolBytesFromString(Ruby runtime, String internedSymbol
}

public static final class SymbolTable {
static final int DEFAULT_INITIAL_CAPACITY = 2048; // *must* be power of 2!
static final int MAXIMUM_CAPACITY = 1 << 30;
static final int DEFAULT_INITIAL_CAPACITY = 1 << 10; // *must* be power of 2!
static final int MAXIMUM_CAPACITY = 1 << 16; // enough for a 64k buckets; if you need more than this, something's wrong
static final float DEFAULT_LOAD_FACTOR = 0.75f;

private final ReentrantLock tableLock = new ReentrantLock();
private volatile SymbolEntry[] symbolTable;
private final ConcurrentHashMap<ByteList, WeakReference<RubySymbol>> bytelistTable = new ConcurrentHashMap<ByteList, WeakReference<RubySymbol>>(100, 0.75f, Runtime.getRuntime().availableProcessors());
private int size;
private int threshold;
private final float loadFactor;
@@ -661,85 +659,66 @@ public SymbolTable(Ruby runtime) {
static class SymbolEntry {
final int hash;
final String name;
final ByteList bytes;
final WeakReference<RubySymbol> symbol;
final SymbolEntry next;
SymbolEntry next;

SymbolEntry(int hash, String name, RubySymbol symbol, SymbolEntry next) {
SymbolEntry(int hash, String name, ByteList bytes, WeakReference<RubySymbol> symbol, SymbolEntry next) {
this.hash = hash;
this.name = name;
this.symbol = new WeakReference(symbol);
this.bytes = bytes;
this.symbol = symbol;
this.next = next;
}
}

public RubySymbol getSymbol(String name) {
int hash = name.hashCode();
SymbolEntry[] table = symbolTable;
int hash = javaStringHashCode(name);
RubySymbol symbol = null;

for (SymbolEntry e = getEntryFromTable(table, hash); e != null; e = e.next) {
for (SymbolEntry e = getEntryFromTable(symbolTable, hash); e != null; e = e.next) {
if (isSymbolMatch(name, hash, e)) {
symbol = e.symbol.get();
if (symbol == null) {
// FIXME: remove weak entry?
}
break;
}
}

if (symbol == null) symbol = createSymbol(name, symbolBytesFromString(runtime, name), hash, table);
if (symbol == null) symbol = createSymbol(name, symbolBytesFromString(runtime, name), hash);

return symbol;
}

public RubySymbol getSymbol(ByteList bytes) {
RubySymbol symbol = null;
WeakReference<RubySymbol> symbolRef = bytelistTable.get(bytes);
if (symbolRef != null) {
symbol = symbolRef.get();
}

if (symbol != null) return symbol;
int hash = javaStringHashCode(bytes);

String name = bytes.toString();
int hash = name.hashCode();
SymbolEntry[] table = symbolTable;

for (SymbolEntry e = getEntryFromTable(table, hash); e != null; e = e.next) {
if (isSymbolMatch(name, hash, e)) {
for (SymbolEntry e = getEntryFromTable(symbolTable, hash); e != null; e = e.next) {
if (isSymbolMatch(bytes, hash, e)) {
symbol = e.symbol.get();
if (symbol == null) {
// FIXME: remove weak entry?
}
break;
}
}

if (symbol == null) {
symbol = createSymbol(name, bytes, hash, table);
bytes = bytes.dup();
symbol = createSymbol(bytes.toString(), bytes, hash);
}

bytelistTable.put(bytes, new WeakReference<RubySymbol>(symbol));

return symbol;
}

public RubySymbol fastGetSymbol(String internedName) {
SymbolEntry[] table = symbolTable;
RubySymbol symbol = null;

for (SymbolEntry e = getEntryFromTable(symbolTable, internedName.hashCode()); e != null; e = e.next) {
if (isSymbolMatch(internedName, e)) {
symbol = e.symbol.get();
if (symbol == null) {
// FIXME: remove weak entry?
}
break;
}
}

if (symbol == null) {
symbol = fastCreateSymbol(internedName, table);
symbol = fastCreateSymbol(internedName);
}

return symbol;
@@ -753,35 +732,48 @@ private static boolean isSymbolMatch(String name, int hash, SymbolEntry entry) {
return hash == entry.hash && name.equals(entry.name);
}

private static boolean isSymbolMatch(ByteList bytes, int hash, SymbolEntry entry) {
return hash == entry.hash && bytes.equals(entry.bytes);
}

private static boolean isSymbolMatch(String internedName, SymbolEntry entry) {
return internedName == entry.name;
}

private RubySymbol createSymbol(String name, ByteList value, int hash, SymbolEntry[] table) {
private RubySymbol createSymbol(final String name, final ByteList value, final int hash) {
ReentrantLock lock;
(lock = tableLock).lock();
try {
int index;
int potentialNewSize = size + 1;
final SymbolEntry[] table = size > threshold ? rehash() : symbolTable;
final int index = hash & (table.length - 1);
RubySymbol symbol = null;

table = potentialNewSize > threshold ? rehash() : symbolTable;

// try lookup again under lock
for (SymbolEntry e = table[index = hash & (table.length - 1)]; e != null; e = e.next) {
if (hash == e.hash && name.equals(e.name)) {
symbol = e.symbol.get();
if (symbol == null) {
// FIXME: remove weak entry?
}
for (SymbolEntry last = null, curr = table[index]; curr != null; curr = curr.next) {
RubySymbol localSymbol = curr.symbol.get();

if (localSymbol == null) {
removeDeadEntry(table, index, last, curr);

// if it's not our entry, proceed to next
if (hash != curr.hash || !name.equals(curr.name)) continue;
}

// update last entry that was either not dead or not the one we want
last = curr;

// if have a matching entry -- even if symbol has gone away -- exit the loop
if (hash == curr.hash && name.equals(curr.name)) {
symbol = localSymbol;
break;
}
}

if (symbol == null) {
String internedName = name.intern();
symbol = new RubySymbol(runtime, internedName, value);
table[index] = new SymbolEntry(hash, internedName, symbol, table[index]);
size = potentialNewSize;
table[index] = new SymbolEntry(hash, internedName, value, new WeakReference(symbol), table[index]);
size++;
// write-volatile
symbolTable = table;
}
@@ -791,31 +783,50 @@ private RubySymbol createSymbol(String name, ByteList value, int hash, SymbolEnt
}
}

private RubySymbol fastCreateSymbol(String internedName, SymbolEntry[] table) {
private void removeDeadEntry(SymbolEntry[] table, int index, SymbolEntry last, SymbolEntry e) {
if (last == null) {
table[index] = e.next; // shift head of bucket
} else {
last.next = e.next; // remove collected bucket entry
}

size--; // reduce current size because we lost one somewhere
}

private RubySymbol fastCreateSymbol(final String internedName) {
ReentrantLock lock;
(lock = tableLock).lock();
try {
int index;
int hash;
int potentialNewSize = size + 1;
final SymbolEntry[] table = size + 1 > threshold ? rehash() : symbolTable;
final int hash = internedName.hashCode();
final int index = hash & (table.length - 1);
RubySymbol symbol = null;

table = potentialNewSize > threshold ? rehash() : symbolTable;

// try lookup again under lock
for (SymbolEntry e = table[index = (hash = internedName.hashCode()) & (table.length - 1)]; e != null; e = e.next) {
if (internedName == e.name) {
symbol = e.symbol.get();
if (symbol == null) {
// FIXME: remove weak entry?
}
for (SymbolEntry last = null, curr = table[index]; curr != null; curr = curr.next) {
RubySymbol localSymbol = curr.symbol.get();

if (localSymbol == null) {
removeDeadEntry(table, index, last, curr);

// if it's not our entry, proceed to next
if (internedName != curr.name) continue;
}

// update last entry that was either not dead or not the one we want
last = curr;

// if have a matching entry -- even if symbol has gone away -- exit the loop
if (internedName == curr.name) {
symbol = localSymbol;
break;
}
}

if (symbol == null) {
symbol = new RubySymbol(runtime, internedName);
table[index] = new SymbolEntry(hash, internedName, symbol, table[index]);
size = potentialNewSize;
table[index] = new SymbolEntry(hash, internedName, symbol.getBytes(), new WeakReference(symbol), table[index]);
size++;
// write-volatile
symbolTable = table;
}
@@ -825,25 +836,6 @@ private RubySymbol fastCreateSymbol(String internedName, SymbolEntry[] table) {
}
}

// backwards-compatibility, but threadsafe now
public RubySymbol lookup(String name) {
int hash = name.hashCode();
SymbolEntry[] table;
RubySymbol symbol = null;

for (SymbolEntry e = (table = symbolTable)[hash & (table.length - 1)]; e != null; e = e.next) {
if (hash == e.hash && name.equals(e.name)) {
symbol = e.symbol.get();
if (symbol == null) {
// FIXME: remove weak entry?
}
break;
}
}

return symbol;
}

public RubySymbol lookup(long id) {
SymbolEntry[] table = symbolTable;
RubySymbol symbol = null;
@@ -872,13 +864,6 @@ public RubyArray all_symbols() {
return array;
}

// not so backwards-compatible here, but no one should have been
// calling this anyway.
@Deprecated
public void store(RubySymbol symbol) {
throw new UnsupportedOperationException();
}

private SymbolEntry[] rehash() {
SymbolEntry[] oldTable = symbolTable;
int oldCapacity = oldTable.length;
@@ -894,44 +879,84 @@ private SymbolEntry[] rehash() {
// We need to guarantee that any existing reads of old Map can
// proceed. So we cannot yet null out each bin.
e = oldTable[i];

if (e != null) {
SymbolEntry next = e.next;
int idx = e.hash & sizeMask;

// Single node on list
if (next == null) {
newTable[idx] = e;
} else {
// Reuse trailing consecutive sequence at same slot
SymbolEntry lastRun = e;
int lastIdx = idx;
for (SymbolEntry last = next;
last != null;
last = last.next) {
int k = last.hash & sizeMask;
if (k != lastIdx) {
lastIdx = k;
lastRun = last;
}
}
newTable[lastIdx] = lastRun;

// Clone all remaining nodes
for (SymbolEntry p = e; p != lastRun; p = p.next) {
RubySymbol symbol = p.symbol.get();
if (symbol != null) {
int k = p.hash & sizeMask;
SymbolEntry n = newTable[k];
newTable[k] = new SymbolEntry(p.hash, p.name, symbol, n);
}
if (e == null) continue;

SymbolEntry next = e.next;
int idx = e.hash & sizeMask;

// Single node on list, reuse it
if (next == null) {
newTable[idx] = e;
} else {
// Reuse trailing consecutive sequence at same slot
SymbolEntry lastRun = e;
int lastIdx = idx;
for (SymbolEntry last = next;
last != null;
last = last.next) {
int k = last.hash & sizeMask;
if (k != lastIdx) {
lastIdx = k;
lastRun = last;
}
}
newTable[lastIdx] = lastRun;

// Clone all remaining nodes
for (SymbolEntry p = e; p != lastRun; p = p.next) {
int k = p.hash & sizeMask;
SymbolEntry n = newTable[k];
newTable[k] = new SymbolEntry(p.hash, p.name, p.bytes, p.symbol, n);
}
}
}
symbolTable = newTable;
return newTable;
}

// backwards-compatibility, but threadsafe now
@Deprecated
public RubySymbol lookup(String name) {
int hash = name.hashCode();
SymbolEntry[] table = symbolTable;
RubySymbol symbol = null;

SymbolEntry e = table[hash & (table.length - 1)];
while (e != null) {
if (hash == e.hash && name.equals(e.name)) {
symbol = e.symbol.get();
if (symbol != null) break;
}
e = e.next;
}

return symbol;
}

// not so backwards-compatible here, but no one should have been
// calling this anyway.
@Deprecated
public void store(RubySymbol symbol) {
throw new UnsupportedOperationException();
}
}

private static int javaStringHashCode(String str) {
return str.hashCode();
}

private static int javaStringHashCode(ByteList iso8859) {
int h = 0;
int length = iso8859.length();
if (h == 0 && length > 0) {
byte val[] = iso8859.getUnsafeBytes();
int begin = iso8859.begin();

for (int i = 0; i < length; i++) {
h = 31 * h + val[begin + i];
}
}
return h;
}

@Override
2 changes: 0 additions & 2 deletions core/src/main/ruby/jruby/java/java_ext/java.io.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
require 'jruby' unless defined? JRuby

class java::io::InputStream
def to_io(opts = nil)
ruby_io = org.jruby.RubyIO.new(JRuby.runtime, self)
4 changes: 0 additions & 4 deletions core/src/test/java/org/jruby/test/TestRubySymbol.java
Original file line number Diff line number Diff line change
@@ -49,16 +49,12 @@ public void setUp() {

public void testSymbolTable() throws Exception {
RubySymbol.SymbolTable st = runtime.getSymbolTable();

assertNull(st.lookup("somename"));

RubySymbol symbol = RubySymbol.newSymbol(runtime, "somename");
assertSame(symbol, st.lookup("somename"));
assertSame(symbol, st.getSymbol("somename"));
assertSame(symbol, st.fastGetSymbol("somename"));

RubySymbol another = st.fastGetSymbol("another_name");
assertSame(another, st.lookup("another_name"));
assertSame(another, st.getSymbol("another_name"));
assertSame(another, st.fastGetSymbol("another_name"));
}