Feature #3478: Excruciatingly slow pathname implementation

• Status: Closed
• Assignee: akr (Akira Tanaka)
• Target version: 2.0.0

[stouset (Stephen Touset)]

=begin
In writing a pure-Ruby git implementation, I discovered with rprof that using Pathname was the source of a huge proportion of my library's running time. Recently, Rails contributor José Valim discovered a similar situation in Rails. Anecdotally, removing Pathname resulted in a speedup from 6s to 3s on a single request. Here's his commit removing some Pathname usage from Rails:

 http://github.com/rails/rails/commit/69abbe893413c99808c8cebd2f1d468c7921c573

I have rewritten Pathname, and the project is on GitHub.

 http://github.com/stouset/pathname3/

I've included the rewrite as a patch. It's backwards-compatible with the existing Pathname class, and passes RubySpec and MRI tests.
=end

[mame (Yusuke Endoh)]

• Assignee set to akr (Akira Tanaka)
• Target version set to 2.0.0

=begin
Hi,

This is not a bug but feature request. This ticket is moved to Feature
tracker.

Your patch might be good, but apparently too big. I guess it will take
long time to review it.

I recommend you to explain essensial changes for performance improvement,
and to split the patch for each change. And, do not remove documents
without reason.

--
Yusuke Endoh [e-mail]
=end

[stouset (Stephen Touset)]

=begin
The patch would be very difficult to do change by change, partly because the entire class is rewritten to descend from String. Suffice it to say, virtually all of the methods in the original Pathname library tried to implement logic themselves, sometimes in a very inefficient manner, rather than falling back upon the well-optimized implementations in File. This rewrite is akin to the switch from CSV to FasterCSV, which I don't believe was submitted as a series of patches. As the code passes all of the related Ruby specs, I wouldn't think this would take too much effort to review and merge in.

The documentation was different because I rewrote the class without the anticipation of having it become part of standard Ruby. I can change the code to mimic the original comments in Pathname without too much effort, if it would help get the patch accepted.
=end

[shyouhei (Shyouhei Urabe)]

=begin
-1. Seems your lib lacks Windows support.
=end

[stouset (Stephen Touset)]

=begin
As I understand it, so does the original. Again, it passes all existing tests.
=end

[shyouhei (Shyouhei Urabe)]

=begin
No, your understanding is wrong.

C:\Users\shyouhei\Documents>C:\ruby\bin\ruby.exe -rpathname -e"p Pathname('C:\').relative_path_from Pathname('D:\')"
C:/ruby/lib/ruby/1.8/pathname.rb:723:in `relative_path_from': different prefix:
"C:\" and "D:\" (ArgumentError)
from -e:1

C:\Users\shyouhei\Documents>C:\ruby\bin\ruby.exe -rpathname3 -e"p Pathname('C:\').relative_path_from Pathname('D:\')"
"../C:\"

=end

[stouset (Stephen Touset)]

=begin
I'll be glad to address those issues, then. Give me a day or two to add some Windows support.
=end

[stouset (Stephen Touset)]

=begin
I haven't forgot about fixing the implementation to support Windows. I had to move apartments earlier than expected, and haven't had the free time to add the necessary fixes.
=end

[naruse (Yui NARUSE)]

• Status changed from Open to Closed

=begin
This problem will be resolved by C implementation of Pathname; see and test trunk.
=end