Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Truffle] Implemented IO.binread #2634

Merged
merged 1 commit into from Mar 1, 2015

Conversation

bjfish
Copy link
Contributor

@bjfish bjfish commented Mar 1, 2015

No description provided.

@bjfish
Copy link
Contributor Author

bjfish commented Mar 1, 2015

@chrisseaton If I want to update a ruby spec, do I just submit a PR to change the specs in this repository? Or are these updated another way? Thanks.

@chrisseaton
Copy link
Contributor

Since Brian deleted the communal RubySpec repository we have been making a few changes to the specs here in this repository, so feel free to do the same. It's probably best to keep the changes to either obvious fixes or additions, and not to refactor code for the sake of it. Soon they'll be merged into a new communal repository at https://github.com/ruby/rubyspec.

@chrisseaton
Copy link
Contributor

This looks like your usual high quality code, but there's some more things you should know about the way we're trying to work.

To implement IO#binread you've written 123 lines of Java. What we'd like to do instead is use these 8 lines of Ruby code from Rubinius 2.4.1 https://github.com/rubinius/rubinius/blob/v2.4.1/kernel/common/io.rb#L262-L269. Here is an example of us doing that: 6285100.

Our implementation of IO really needs someone to tackle it from scratch and work out what primitives we need to implement and how to integrate the rest from Rubinius. This is going to be quite a lot of architectural work so I wouldn't recommend it for someone outside the core team at the moment, who have more understanding of some of the very subtle issues of working with Truffle and Graal.

However this is good working code, so I will gratefully merge it.

The other classes you've been working on so far, such as Symbol and String are a little lower level so we're keeping a lot of that in Java (also Fixnum, Proc etc).

We really appreciate your commits - we're pretty surprised you seem to have grasped so much of Truffle so quickly! What's motivating you? Are you trying to get some specific code running? If you're just experimenting we could help you find an area of the implementation that needs some work and where you could have a big impact. If you are really keen we could talk to you about what it would take to get IO working.

Catch us on #jruby IRC (EU and west coast time zones).

chrisseaton added a commit that referenced this pull request Mar 1, 2015
[Truffle] Implemented IO.binread
@chrisseaton chrisseaton merged commit e5ed970 into jruby:master Mar 1, 2015
@bjfish
Copy link
Contributor Author

bjfish commented Mar 1, 2015

@chrisseaton

What's motivating you?

I'd like to see a faster ruby and the benchmarks look promising so far. Also, I'm interested in learning more about interperters/JIT and find Truffle's approach very interesting. Especially, the post about interperting C code.

Are you trying to get some specific code running?

Right now, I'm just experimenting. I would be interested in trying to get some specific code running like json_pure (to do a benchmark) or sinatra (to attempt to run web stuff).

If you're just experimenting we could help you find an area of the implementation that needs some work and where you could have a big impact.

Let me know what I should focus on. Thanks.

@chrisseaton
Copy link
Contributor

json_pure would be very helpful - it's on our todo list anyway. json_pure depends on strscan, so the first step would be to make sure that works. I've imported the Rubinius implementation and untagged the specs which already pass in 86681e5.

You should just be able to work through the rest of the strscan specs until they're all working. You shouldn't have to modify strscan at all, but there are probably missing methods you'll need to implement in String and Regex.

Then we can look at json_pure. It will be a while before you can do any meaningful benchmarks, as our String and Regex implementations are a bit weak at the moment (they often work by converting to JRuby objects, doing the operation, and then back to Truffle objects).

@chrisseaton chrisseaton modified the milestone: truffle-dev May 4, 2015
@enebo enebo added this to the Non-Release milestone Dec 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants