Truffle encoding #2765

nirvdrum · 2015-03-27T02:15:30Z

This is a bit large and I had to do some interesting things to init Rubinius properly. I'd appreciate another set of eyes before I merge it. @chrisseaton @eregon

…String.

…ell. While we're using Rubinius's encoding subsystem substantially, we still delegate to JRuby methods in various places. JRuby's runtime is unaware of any encoding values set via Rubinius and as a result, we can generate strings with incorrect encodings if we don't take care to update JRuby's runtime as well.

…onverterPrimitiveNodes.

…ive.

…ava-based ones if they exist.

…code! from Rubinius. This commit also fixes several issues in our encoding implementation that surfaced through greater usage of Encoding and Encoding::Converter.

…ort.

chrisseaton · 2015-03-27T02:27:36Z

truffle/src/main/java/org/jruby/truffle/nodes/core/EncodingNodes.java

+                    null,
+                    getContext().makeString("filesystem"),
+                    indexLookup(encodings, filesystemEncoding)
+            );


Can you create some kind of createTuple helper here?

eregon · 2015-03-27T13:13:50Z

truffle/src/main/ruby/core/shims.rb

+      self.default_internal_jruby = enc
+    end
+  end
+end


I like this

eregon · 2015-03-27T13:38:44Z

truffle/src/main/ruby/core/rubinius/delta/converter_paths.rb does not look pretty with its 7000+ assignments to a Hash. Do we have this map already somewhere in JRuby?
Also most of them are "go through UTF-8", not really sure of the gain to save that path in memory.

eregon · 2015-03-27T13:40:43Z

truffle/src/main/ruby/core/rubinius/common/encoding.rb

+      @paths = {}
+      @load_cache = true
+      @cache_valid = false
+      @transcoders_count = TranscodingMap.size


Ah here, I see. Could be worth to make a private class method setup that initialize to non-trivial values. But probably good enough for now.

I was trying to avoid modifying their source altogether since it would make future updates easier. But I'm open to other options.

Yeah, this is good for now I think.

…'s an internal API call.

Truffle encoding

eregon · 2015-03-27T14:20:03Z

Startup suffers quite a bit though ...

$ for i in {1..10}; do; jt run -e 'puts :hi'; done
Before:
  4.46s user 0.14s system 160% cpu 2.872 total
  4.40s user 0.14s system 159% cpu 2.845 total
  4.45s user 0.13s system 159% cpu 2.875 total
After:
  10.18s user 0.22s system 169% cpu 6.145 total
  10.18s user 0.22s system 164% cpu 6.308 total
  9.91s user 0.24s system 164% cpu 6.166 total

chrisseaton · 2015-03-27T14:25:16Z

Wow! Normally I'd say not to worry about it as it's not peak performance - but 10s is a bit insane. We risk people trying Truffle out getting a very bad impression. It might be the hash operations - we never resize remember - so I'll see if I can optimise the methods and specialisations used in converter_paths.rb.

nirvdrum · 2015-03-27T14:33:48Z

It looks like that converter_paths load is to blame after all. I'll work on fixing that now. While big, it's a bit disconcerting that interpreted hash updates are that slow. That might warrant additional investigation.

eregon · 2015-03-27T14:47:24Z

Just parsing that file might be slow, Rbx precompiles to .rbc so that might motivate their approach.

eregon · 2015-03-29T09:29:42Z

@nirvdrum Seems much better after 53683ba:

  4.74s user 0.14s system 163% cpu 2.990 total
  4.68s user 0.15s system 161% cpu 2.999 total
  4.73s user 0.13s system 159% cpu 3.042 total

nirvdrum added 21 commits March 26, 2015 11:33

[Truffle] Initial import of Rubinius's encoding system.

e74f51a

[Truffle] Added the default internal encoding to the encoding map.

06c0938

[Truffle] Make debugging RubyEncodings a bit easier by providing a to…

3e6214d

…String.

[Truffle] Properly initialize internal state for Rubinius's Encoding.

b79d61f

[Truffle] Use the correct lookup index for predefined encoding aliases.

a77012d

[Truffle] Added the 'filesystem' encoding to the encoding map.

bda1c2a

[Truffle] Encoding::EncodingMap keys need to be uppercase.

68ddf9d

[Truffle] Use the Java-based Encoding.locale_charmap.

87327da

[Truffle] Untagged a bunch of Encoding and Encoding::Converter specs.

6781031

[Truffle] Filled in more Encoding::Converter functionality.

df24452

[Truffle] Created EncodingConverterPrimitiveNodes.

349340a

[Truffle] Moved some stuff out of EncodingPrimitiveNodes to EncodingC…

366f5ae

…onverterPrimitiveNodes.

[Truffle] Implemented the encoding_converter_primitive_errinfo primit…

89afac2

…ive.

[Truffle] Cleaned up a bit now that Rubinius methods won't override J…

4762cc4

…ava-based ones if they exist.

[Truffle] The Encoding::TranscodingMap keys must be upper-cased.

63f9178

[Truffle] Fixed initializiation of RubyConverter.

20c6a62

[Truffle] Moved String#encode out to Rubinius and pulled in String#en…

b28adbf

…code! from Rubinius. This commit also fixes several issues in our encoding implementation that surfaced through greater usage of Encoding and Encoding::Converter.

[Truffle] Untagged more Encoding and Encoding::Converter specs.

377baba

[Truffle] Untagged String specs passing due to improved encoding supp…

49d3785

…ort.

[Truffle] Removed unnecessary semi-colons.

46f7a78

chrisseaton reviewed Mar 27, 2015
View reviewed changes

[Truffle] Added a factory method for creating Rubinius tuples.

baf122f

eregon reviewed Mar 27, 2015
View reviewed changes

truffle/src/main/ruby/core/shims.rb

self.default_internal_jruby = enc

end

end

end

Copy link

Member

eregon Mar 27, 2015

Choose a reason for hiding this comment

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

I like this

eregon reviewed Mar 27, 2015
View reviewed changes

[Truffle] Made Encoding::Converter#initialize_jruby private, since it…

acca8ae

…'s an internal API call.

nirvdrum added a commit that referenced this pull request Mar 27, 2015

Merge pull request #2765 from jruby/truffle_encoding

da248f2

Truffle encoding

nirvdrum merged commit da248f2 into master Mar 27, 2015

nirvdrum deleted the truffle_encoding branch March 27, 2015 14:10

chrisseaton modified the milestone: truffle-dev May 4, 2015

enebo added this to the Non-Release milestone Dec 7, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Truffle encoding #2765

Truffle encoding #2765

nirvdrum commented Mar 27, 2015

chrisseaton Mar 27, 2015

eregon Mar 27, 2015

eregon commented Mar 27, 2015

eregon Mar 27, 2015

nirvdrum Mar 27, 2015

eregon Mar 27, 2015

eregon commented Mar 27, 2015

chrisseaton commented Mar 27, 2015

nirvdrum commented Mar 27, 2015

eregon commented Mar 27, 2015

eregon commented Mar 29, 2015

Truffle encoding #2765

Truffle encoding #2765

Conversation

nirvdrum commented Mar 27, 2015

chrisseaton Mar 27, 2015

Choose a reason for hiding this comment

eregon Mar 27, 2015

Choose a reason for hiding this comment

eregon commented Mar 27, 2015

eregon Mar 27, 2015

Choose a reason for hiding this comment

nirvdrum Mar 27, 2015

Choose a reason for hiding this comment

eregon Mar 27, 2015

Choose a reason for hiding this comment

eregon commented Mar 27, 2015

chrisseaton commented Mar 27, 2015

nirvdrum commented Mar 27, 2015

eregon commented Mar 27, 2015

eregon commented Mar 29, 2015