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] New IO implementation #2866

Merged
merged 27 commits into from Apr 23, 2015
Merged

[Truffle] New IO implementation #2866

merged 27 commits into from Apr 23, 2015

Conversation

chrisseaton
Copy link
Contributor

This reimplements Truffle's File and IO classes using Rubinius, on top of a few Rubinius primitives and some POSIX calls. There's now no reference to File or IO in the Java code at all.

This gives patch alone gives us 761 new passing specs (about 5% of the entire core specs), but it also opens the door for the rest of IO and things like Socket.

The primitives use ruby() a lot, mainly to read and write instance variables, which we might want to remove sooner rather than later.

There is also a lot of copying going on here. I think puts will copy at least four times. The Rubinius primitives also rely on stack allocation of large buffers being efficient, which does not translate well into Java.

@eregon @nirvdrum @thomaswue

@chrisseaton chrisseaton added this to the truffle-dev milestone Apr 22, 2015
@chrisseaton chrisseaton self-assigned this Apr 22, 2015
//if (!state -> check_async(calling_environment))
// return NULL;
//io -> ensure_open(state);
getContext().getSafepointManager().poll(this);
Copy link
Member

Choose a reason for hiding this comment

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

If this actually works on STDIN, this will be awesome!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

STDIN.each_line do |line|
  puts line
end

With both tty and piped input works. Is that what you meant?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, exactly 👍

Copy link
Member

Choose a reason for hiding this comment

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

I mean, the ability to interrupt reading from STDIN is what interests me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, I'm afraid it doesn't appear to work at the moment. Can you interrupt all reads in a process somehow?

Conflicts:
	truffle/src/main/ruby/core/rubinius/common/kernel.rb
	truffle/src/main/ruby/core/shims.rb
chrisseaton added a commit that referenced this pull request Apr 23, 2015
[Truffle] New IO implementation
@chrisseaton chrisseaton merged commit 9fad714 into master Apr 23, 2015
@chrisseaton
Copy link
Contributor Author

Merged, but still open for comments

public EachNode(RubyContext context, SourceSection sourceSection) {
super(context, sourceSection);
eachSymbol = getContext().getSymbolTable().getSymbol("each");
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we use getContext().newSymbol() almost exclusively. It might make future refactorings easier to stick with that.

Copy link
Member

Choose a reason for hiding this comment

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

Didn't know about that. I would expect it to be getContext().getSymbol() since this will likely not create a new Symbol everytime.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's perhaps a bit misleadingly named. It just does return symbolTable.getSymbol(name, ASCIIEncoding.INSTANCE);, which will return a cached Symbol if it already exists or creates a new one and populates the cache otherwise.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 5c53e3c

@chrisseaton chrisseaton deleted the truffle-io branch April 29, 2015 20:36
@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

4 participants