Skip to content

Commit

Permalink
Showing 4 changed files with 89 additions and 45 deletions.
20 changes: 14 additions & 6 deletions truffle/src/main/java/org/jruby/truffle/nodes/core/QueueNodes.java
Original file line number Diff line number Diff line change
@@ -17,15 +17,17 @@
import com.oracle.truffle.api.dsl.Specialization;
import com.oracle.truffle.api.object.DynamicObject;
import com.oracle.truffle.api.source.SourceSection;

import org.jruby.runtime.Visibility;
import org.jruby.truffle.nodes.RubyNode;
import org.jruby.truffle.nodes.cast.BooleanCastWithDefaultNodeGen;
import org.jruby.truffle.runtime.RubyContext;
import org.jruby.truffle.runtime.control.RaiseException;
import org.jruby.truffle.runtime.layouts.Layouts;
import org.jruby.truffle.runtime.subsystems.ThreadManager.BlockingAction;
import org.jruby.util.unsafe.UnsafeHolder;
import org.jruby.truffle.runtime.util.MethodHandleUtils;

import java.lang.invoke.MethodHandle;
import java.util.concurrent.BlockingQueue;
import java.util.concurrent.LinkedBlockingQueue;
import java.util.concurrent.TimeUnit;
@@ -230,21 +232,27 @@ public Object marshal_dump(DynamicObject self) {
@CoreMethod(names = "num_waiting")
public abstract static class NumWaitingNode extends CoreMethodArrayArgumentsNode {

private static final long LOCK_FIELD_OFFSET = UnsafeHolder.fieldOffset(LinkedBlockingQueue.class, "takeLock");
private static final long NOT_EMPTY_CONDITION_FIELD_OFFSET = UnsafeHolder.fieldOffset(LinkedBlockingQueue.class, "notEmpty");
private static final MethodHandle TAKE_LOCK_FIELD_GETTER = MethodHandleUtils.getPrivateGetter(LinkedBlockingQueue.class, "takeLock");
private static final MethodHandle NOT_EMPTY_CONDITION_FIELD_GETTER = MethodHandleUtils.getPrivateGetter(LinkedBlockingQueue.class, "notEmpty");

This comment has been minimized.

Copy link
@chrisseaton

chrisseaton Feb 7, 2016

Contributor

Is there no better way to do this? I'm trying to limit use of things like unsafe and reflection where possible for a cleaner implementation.

This comment has been minimized.

Copy link
@eregon

eregon Feb 11, 2016

Author Member

No, it's a private field and we need it to implement num_waiting, unless we would keep track ourselves of 1-2 additional fields which seems worse for the implementation complexity.
The only "right" solution would be to have our own Queue/SizedQueue implementation, which I think long-term is interesting but right now not worth it.
Is it causing any problem in practice?

This comment has been minimized.

Copy link
@chrisseaton

chrisseaton Feb 11, 2016

Contributor

I'm was just trying to encapsulate any non-standard behaviour we rely on. It's ok for now. I might wrapt it up into a class so the MethodHandle stuff is hidden.

This comment has been minimized.

Copy link
@chrisseaton

chrisseaton Feb 11, 2016

Contributor

I encapsulated in 68f93a1...b435b4f.


public NumWaitingNode(RubyContext context, SourceSection sourceSection) {
super(context, sourceSection);
}

@SuppressWarnings("restriction")
@Specialization
public int num_waiting(DynamicObject self) {
final BlockingQueue<Object> queue = Layouts.QUEUE.getQueue(self);

final LinkedBlockingQueue<Object> linkedBlockingQueue = (LinkedBlockingQueue<Object>) queue;
final ReentrantLock lock = (ReentrantLock) UnsafeHolder.U.getObject(linkedBlockingQueue, LOCK_FIELD_OFFSET);
final Condition notEmptyCondition = (Condition) UnsafeHolder.U.getObject(linkedBlockingQueue, NOT_EMPTY_CONDITION_FIELD_OFFSET);

final ReentrantLock lock;
final Condition notEmptyCondition;
try {
lock = (ReentrantLock) TAKE_LOCK_FIELD_GETTER.invokeExact(linkedBlockingQueue);
notEmptyCondition = (Condition) NOT_EMPTY_CONDITION_FIELD_GETTER.invokeExact(linkedBlockingQueue);
} catch (Throwable e) {
throw new RuntimeException(e);
}

getContext().getThreadManager().runUntilResult(this, new BlockingAction<Boolean>() {
@Override
Original file line number Diff line number Diff line change
@@ -17,15 +17,16 @@
import com.oracle.truffle.api.dsl.Specialization;
import com.oracle.truffle.api.object.DynamicObject;
import com.oracle.truffle.api.source.SourceSection;

import org.jruby.runtime.Visibility;
import org.jruby.truffle.nodes.RubyNode;
import org.jruby.truffle.nodes.cast.BooleanCastWithDefaultNodeGen;
import org.jruby.truffle.runtime.RubyContext;
import org.jruby.truffle.runtime.control.RaiseException;
import org.jruby.truffle.runtime.layouts.Layouts;
import org.jruby.truffle.runtime.subsystems.ThreadManager.BlockingAction;
import org.jruby.util.unsafe.UnsafeHolder;

import org.jruby.truffle.runtime.util.MethodHandleUtils;
import java.lang.invoke.MethodHandle;
import java.util.concurrent.ArrayBlockingQueue;
import java.util.concurrent.BlockingQueue;
import java.util.concurrent.locks.Condition;
@@ -264,23 +265,30 @@ public DynamicObject clear(DynamicObject self) {
@CoreMethod(names = "num_waiting")
public abstract static class NumWaitingNode extends CoreMethodArrayArgumentsNode {

private static final long LOCK_FIELD_OFFSET = UnsafeHolder.fieldOffset(ArrayBlockingQueue.class, "lock");
private static final long NOT_EMPTY_CONDITION_FIELD_OFFSET = UnsafeHolder.fieldOffset(ArrayBlockingQueue.class, "notEmpty");
private static final long NOT_FULL_CONDITION_FIELD_OFFSET = UnsafeHolder.fieldOffset(ArrayBlockingQueue.class, "notFull");
private static final MethodHandle LOCK_FIELD_GETTER = MethodHandleUtils.getPrivateGetter(ArrayBlockingQueue.class, "lock");
private static final MethodHandle NOT_EMPTY_CONDITION_FIELD_GETTER = MethodHandleUtils.getPrivateGetter(ArrayBlockingQueue.class, "notEmpty");
private static final MethodHandle NOT_FULL_CONDITION_FIELD_GETTER = MethodHandleUtils.getPrivateGetter(ArrayBlockingQueue.class, "notFull");

public NumWaitingNode(RubyContext context, SourceSection sourceSection) {
super(context, sourceSection);
}

@SuppressWarnings("restriction")
@Specialization
public int num_waiting(DynamicObject self) {
final BlockingQueue<Object> queue = Layouts.SIZED_QUEUE.getQueue(self);

final ArrayBlockingQueue<Object> arrayBlockingQueue = (ArrayBlockingQueue<Object>) queue;
final ReentrantLock lock = (ReentrantLock) UnsafeHolder.U.getObject(arrayBlockingQueue, LOCK_FIELD_OFFSET);
final Condition notEmptyCondition = (Condition) UnsafeHolder.U.getObject(arrayBlockingQueue, NOT_EMPTY_CONDITION_FIELD_OFFSET);
final Condition notFullCondition = (Condition) UnsafeHolder.U.getObject(arrayBlockingQueue, NOT_FULL_CONDITION_FIELD_OFFSET);

final ReentrantLock lock;
final Condition notEmptyCondition;
final Condition notFullCondition;
try {
lock = (ReentrantLock) LOCK_FIELD_GETTER.invokeExact(arrayBlockingQueue);
notEmptyCondition = (Condition) NOT_EMPTY_CONDITION_FIELD_GETTER.invokeExact(arrayBlockingQueue);
notFullCondition = (Condition) NOT_FULL_CONDITION_FIELD_GETTER.invokeExact(arrayBlockingQueue);
} catch (Throwable e) {
throw new RuntimeException(e);
}

getContext().getThreadManager().runUntilResult(this, new BlockingAction<Boolean>() {
@Override
34 changes: 4 additions & 30 deletions truffle/src/main/java/org/jruby/truffle/nodes/ext/ZlibNodes.java
Original file line number Diff line number Diff line change
@@ -14,6 +14,7 @@
import com.oracle.truffle.api.dsl.Specialization;
import com.oracle.truffle.api.object.DynamicObject;
import com.oracle.truffle.api.source.SourceSection;

import org.jruby.truffle.nodes.core.CoreClass;
import org.jruby.truffle.nodes.core.CoreMethod;
import org.jruby.truffle.nodes.core.CoreMethodArrayArgumentsNode;
@@ -22,14 +23,11 @@
import org.jruby.truffle.runtime.control.RaiseException;
import org.jruby.truffle.runtime.core.StringOperations;
import org.jruby.truffle.runtime.layouts.Layouts;
import org.jruby.truffle.runtime.util.MethodHandleUtils;
import org.jruby.util.ByteList;
import org.jruby.util.StringSupport;

import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodHandles;
import java.lang.reflect.Field;
import java.security.AccessController;
import java.security.PrivilegedAction;
import java.util.zip.CRC32;
import java.util.zip.DataFormatException;
import java.util.zip.Deflater;
@@ -157,31 +155,7 @@ public DynamicObject inflate(DynamicObject message) {
@CoreMethod(names = "adler32", isModuleFunction = true, required = 0, optional = 2, lowerFixnumParameters = 1)
public abstract static class Adler32Node extends CoreMethodArrayArgumentsNode {

private static final MethodHandle ADLER_PRIVATE_FIELD;

static {
try {
final Field f = AccessController.doPrivileged(new PrivilegedAction<Field>() {
@Override
public Field run() {
final Field f;

try {
f = Adler32.class.getDeclaredField("adler");
f.setAccessible(true);

return f;
} catch (NoSuchFieldException e) {
throw new RuntimeException(e);
}
}
});

ADLER_PRIVATE_FIELD = MethodHandles.lookup().unreflectSetter(f);
} catch (IllegalAccessException e) {
throw new RuntimeException(e);
}
}
private static final MethodHandle ADLER_FIELD_SETTER = MethodHandleUtils.getPrivateSetter(Adler32.class, "adler");

public Adler32Node(RubyContext context, SourceSection sourceSection) {
super(context, sourceSection);
@@ -208,7 +182,7 @@ public long adler32(DynamicObject string, int adler) {
final Adler32 adler32 = new Adler32();

try {
ADLER_PRIVATE_FIELD.invoke(adler32, adler);
ADLER_FIELD_SETTER.invokeExact(adler32, adler);
} catch (Throwable e) {
throw new RuntimeException(e);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/*
* Copyright (c) 2015 Oracle and/or its affiliates. All rights reserved. This
* code is released under a tri EPL/GPL/LGPL license. You can use it,
* redistribute it and/or modify it under the terms of the:
*
* Eclipse Public License version 1.0
* GNU General Public License version 2
* GNU Lesser General Public License version 2.1
*/
package org.jruby.truffle.runtime.util;

import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodHandles;
import java.lang.reflect.Field;
import java.security.AccessController;
import java.security.PrivilegedAction;

public abstract class MethodHandleUtils {

public static MethodHandle getPrivateGetter(Class<?> klass, String fieldName) {
final Field field = getPrivateField(klass, fieldName);
try {
return MethodHandles.lookup().unreflectGetter(field);
} catch (IllegalAccessException e) {
throw new RuntimeException(e);
}
}

public static MethodHandle getPrivateSetter(final Class<?> klass, final String fieldName) {
final Field field = getPrivateField(klass, fieldName);
try {
return MethodHandles.lookup().unreflectSetter(field);
} catch (IllegalAccessException e) {
throw new RuntimeException(e);
}
}

private static Field getPrivateField(final Class<?> klass, final String fieldName) {
return AccessController.doPrivileged(new PrivilegedAction<Field>() {

This comment has been minimized.

Copy link
@nirvdrum

nirvdrum Oct 7, 2015

Contributor

The docs for doPriviliged specifically say you shouldn't make a factory method for this sort of thing. If we're not concerned about that, that's fine, but maybe having a comment would be helpful for anyone else looking at it.

This comment has been minimized.

Copy link
@eregon

eregon Oct 7, 2015

Author Member

It seems we don't even need the doPrivileged() as we expect callers to have sufficient permissions.

This comment has been minimized.

Copy link
@nirvdrum

nirvdrum Oct 7, 2015

Contributor

If you want to remove it, you'll need to annotate FindBugs as well. Not having it originally triggered a FindBugs failure.

@Override
public Field run() {
final Field field;
try {
field = klass.getDeclaredField(fieldName);
field.setAccessible(true);
return field;
} catch (NoSuchFieldException e) {
throw new RuntimeException(e);
}
}
});
}

}

0 comments on commit d06d163

Please sign in to comment.