Skip to content

Commit

Permalink
[Truffle] Re-implement String#include? using Rubinius.
Browse files Browse the repository at this point in the history
  • Loading branch information
chrisseaton committed Jan 5, 2015
1 parent 9e53642 commit 77fd248
Show file tree
Hide file tree
Showing 6 changed files with 108 additions and 3 deletions.
Expand Up @@ -737,7 +737,7 @@ public int getByte(RubyString string, int index) {
}
}

@CoreMethod(names = "include?", required = 1)
@CoreMethod(names = "__include?", required = 1)

This comment has been minimized.

Copy link
@chrisseaton

chrisseaton Jan 5, 2015

Author Contributor

Sorry, leaving this here was a mistake - I've removed this node now.

public abstract static class IncludeQueryNode extends CoreMethodNode {

public IncludeQueryNode(RubyContext context, SourceSection sourceSection) {
Expand Down
Expand Up @@ -14,6 +14,7 @@
import com.oracle.truffle.api.utilities.ConditionProfile;
import org.jruby.truffle.runtime.RubyContext;
import org.jruby.truffle.runtime.core.*;
import org.jruby.util.StringSupport;

/**
* Rubinius primitives associated with the Ruby {@code String} class.
Expand Down Expand Up @@ -64,4 +65,30 @@ public RubyString stringToF(RubyString string) {

}

@RubiniusPrimitive(name = "string_index")
public static abstract class StringIndexPrimitiveNode extends RubiniusPrimitiveNode {

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

public StringIndexPrimitiveNode(StringIndexPrimitiveNode prev) {
super(prev);
}

@Specialization
public Object stringIndex(RubyString string, RubyString pattern, int start) {
final int index = StringSupport.index(string, string.getBytes(), string.length(),
pattern, pattern.getBytes(), pattern.length(),
start, string.getBytes().getEncoding());

if (index == -1) {
return getContext().getCoreLibrary().getNilObject();
}

return index;
}

}

This comment has been minimized.

Copy link
@chrisseaton

chrisseaton Jan 5, 2015

Author Contributor

This is the implementation of the new primitive.


}
2 changes: 2 additions & 0 deletions core/src/main/ruby/jruby/truffle/core.rb
Expand Up @@ -27,6 +27,7 @@

require_relative 'core/rubinius/kernel/bootstrap/time'
require_relative 'core/rubinius/kernel/bootstrap/type'
require_relative 'core/rubinius/kernel/bootstrap/string'

require_relative 'core/rubinius/kernel/common/kernel'
require_relative 'core/rubinius/kernel/common/array'
Expand All @@ -39,5 +40,6 @@
require_relative 'core/rubinius/kernel/common/integer'
require_relative 'core/rubinius/kernel/common/float'
require_relative 'core/rubinius/kernel/common/numeric'
require_relative 'core/rubinius/kernel/common/string'

This comment has been minimized.

Copy link
@chrisseaton

chrisseaton Jan 5, 2015

Author Contributor

We then have to add the new new files to the core.rb file, which is the only one that the VM loads.

require_relative 'core/shims'
@@ -0,0 +1,36 @@
# Copyright (c) 2007-2014, Evan Phoenix and contributors
# All rights reserved.
#
# Redistribution and use in source and binary forms, with or without
# modification, are permitted provided that the following conditions are met:
#
# * Redistributions of source code must retain the above copyright notice, this
# list of conditions and the following disclaimer.
# * Redistributions in binary form must reproduce the above copyright notice
# this list of conditions and the following disclaimer in the documentation
# and/or other materials provided with the distribution.
# * Neither the name of Rubinius nor the names of its contributors
# may be used to endorse or promote products derived from this software
# without specific prior written permission.
#
# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
# DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE
# FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
# SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
# CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

# Only part of Rubinius' string.rb

class String

def find_string(pattern, start)
Rubinius.primitive :string_index
raise PrimitiveFailure, "String#find_string primitive failed"
end

This comment has been minimized.

Copy link
@chrisseaton

chrisseaton Jan 5, 2015

Author Contributor

This is from kernel/bootstrap/string.rb, imported in a similar way as the file below.


end
@@ -0,0 +1,42 @@
# Copyright (c) 2007-2014, Evan Phoenix and contributors
# All rights reserved.
#
# Redistribution and use in source and binary forms, with or without
# modification, are permitted provided that the following conditions are met:
#
# * Redistributions of source code must retain the above copyright notice, this
# list of conditions and the following disclaimer.
# * Redistributions in binary form must reproduce the above copyright notice
# this list of conditions and the following disclaimer in the documentation
# and/or other materials provided with the distribution.
# * Neither the name of Rubinius nor the names of its contributors
# may be used to endorse or promote products derived from this software
# without specific prior written permission.
#
# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
# DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE
# FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
# SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
# CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

# Only part of Rubinius' string.rb

class String

def include?(needle)
if needle.kind_of? Fixnum
needle = needle % 256
str_needle = needle.chr
else
str_needle = StringValue(needle)
end

!!find_string(str_needle, 0)
end

This comment has been minimized.

Copy link
@chrisseaton

chrisseaton Jan 5, 2015

Author Contributor

I took this method from kernel/common/string.rb in Rubinius 2.1.4 and added the copyright message. I also noted that it's not a full copy of the file (so merging is easier if we want to do that).

This comment has been minimized.

Copy link
@nirvdrum

nirvdrum Jan 5, 2015

Contributor

Do you mean 2.4.1? Or did we roll back?

This comment has been minimized.

Copy link
@chrisseaton

chrisseaton Jan 5, 2015

Author Contributor

Yeah 2.4.1. Rubinius are planning to restructure their kernel significantly (with multiple dispatch and things) so best not to pull casually from master and work off a firm commit.


end
2 changes: 0 additions & 2 deletions spec/truffle/tags/core/string/include_tags.txt

This file was deleted.

4 comments on commit 77fd248

@chrisseaton
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eregon @nirvdrum this is an example of how to implement a new method using code from Rubinius. I took a method that @nirvdrum implemented recently, String#include? and re-implemented it using Rubinius.

This should be the first approach to implementing any new method you need to get something working. We can have exceptions for code that is critical for performance, but unless you're running a benchmark you can't make that decision, and even then we should first look to improve how the VM uses the primitives rather than remove them. I think we can also make exceptions if your are basically just implementing a shim to get stuff working and the Rubinius code would actually be far more complicated, and if the Rubinius model is fundamentally incompatible with the JVM (things like FFI calls).

@eregon
Copy link
Member

@eregon eregon commented on 77fd248 Jan 6, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!
As an aside, why are Rubinius FFI calls fundamentally incompatible?

@chrisseaton
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well maybe I put it too strongly, but I think it is unlikely to be the best approach to start simulating memory pointers and sys calls and things - for example their implementation of Math#frexp https://github.com/rubinius/rubinius/blob/master/kernel/common/math.rb#L122-L128. Having said that, I might try that approach for socket.

@eregon
Copy link
Member

@eregon eregon commented on 77fd248 Jan 7, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, for some basic stuff it might not make sense to follow that (although for compat it potentially could I guess).

Please sign in to comment.