Skip to content

Commit

Permalink
[Truffle] Introduce MethodHandleUtils and use MethodHandle to access …
Browse files Browse the repository at this point in the history 
…private fields.
@eregon
eregon committed Oct 7, 2015
1 parent 797e181 commit d06d163
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
View file
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.

Sign in to view

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.

Sign in to view

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.

Sign in to view

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.

Sign in to view

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
26 changes: 17 additions & 9 deletions truffle/src/main/java/org/jruby/truffle/nodes/core/SizedQueueNodes.java
View file
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
View file
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);
}
54 changes: 54 additions & 0 deletions truffle/src/main/java/org/jruby/truffle/runtime/util/MethodHandleUtils.java
View file
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.

Sign in to view

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.

Sign in to view

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.

Sign in to view

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.