Issue #6836 has been reported by luislavena (Luis Lavena). ---------------------------------------- Bug #6836: Improve File.expand_path performance in Windows https://bugs.ruby-lang.org/issues/6836 Author: luislavena (Luis Lavena) Status: Assigned Priority: Normal Assignee: nobu (Nobuyoshi Nakada) Category: core Target version: 2.0.0 ruby -v: ruby 2.0.0dev (2012-08-04 trunk 36616) [i386-mingw32] =begin (original write up in markdown here: https://gist.github.com/3242245) == Background While investigating the performance issues of (({File.expand_path})) on Windows, Usaku Nakamura and Nobuyoshi Nakada on [ruby-core:39504] pointed out that due security concerns, accessing files on Windows required normalized paths. This was covered in the security update of March 2008, WEBrick file-access vulnerability [1]. After closer inspection of WEBrick code (mentioned by the security update), I noticed it uses (({File.expand_path})) to perform the heavy lifting of path normalization in the request. The code around this can be inspected in (({prevent_directory_traversal}))[2] and (({shift_path_info}))[3] methods. This approach performs a hit into the filesystem, contrary to its implementation in any other operating system. (({File.expand_path})) is heavily used by (({require})), which result in slow application startup, depending on the application size or number of gems it depends on. Stepping back for a second, we can see that the security issue is around WEBrick and the way it determines (({path_info})) absoluteness. It is also clear that to solve WEBrick security issue, a tax has been applied to the entire Ruby ecosystem, penalizing startup performance. With Hiroshi Shirosaki's help, we worked on a patch that: * Limit filesystem hit only to WEBrick, using Windows' GetLongPathName [4]. * Use a Windows-specific API to normalize paths * Improve encoding support. What started as an experiment named Fenix [5] has shown great results on a variety of systems. This patch has been integrated in TheCodeShop [6] releases of Ruby 1.9.3 and tested by Ruby-Core developers Hiroshi Shirosaki and myself. == Performance To demonstrate the benefits of this patch, I've used measurements [7] project and both (({core_require_empty})) and (({core_require_nested})) workloads, obtaining the following results: ruby 2.0.0dev (2012-08-03 trunk 36611) [i386-mingw32] Rehearsal ------------------------------------------------------ core_require_empty 1.186000 3.385000 4.571000 ( 4.676267) --------------------------------------------- total: 4.571000sec user system total real core_require_empty 1.217000 3.385000 4.602000 ( 4.643266) Rehearsal ------------------------------------------------------- core_require_nested 1.514000 3.760000 5.274000 ( 5.305303) ---------------------------------------------- total: 5.274000sec user system total real core_require_nested 1.466000 3.713000 5.179000 ( 5.233300) And with patch applied: ruby 2.0.0dev (2012-08-03 trunk 36611) [i386-mingw32] Rehearsal ------------------------------------------------------ core_require_empty 0.765000 1.077000 1.842000 ( 1.887603) --------------------------------------------- total: 1.842000sec user system total real core_require_empty 0.717000 1.123000 1.840000 ( 1.887603) Rehearsal ------------------------------------------------------- core_require_nested 0.717000 1.670000 2.387000 ( 2.480405) ---------------------------------------------- total: 2.387000sec user system total real core_require_nested 0.890000 1.528000 2.418000 ( 2.496004) Benchmarks were performed all on the same hardware and OS: * CPU: Core 2 Duo T7500 @ 2.20Ghz * RAM: 4GB * HDD: 1.5GB RAMdisk (ImDisk) * OS: Windows 7 Ultimate x64 All tests associated (both File and WEBrick ones) pass. Additional tests that exercise specific aspects of new function were added. Patch has been tested also against: * Visual Studio build of Ruby * Ubuntu 12.04 * Mac OSX And the patch didn't affect either build or tests of it. === Real life impact: Rails The biggest Ruby project affected by this is Rails applications. An empty Rails application on startup requires more than 700 files from different gems: V:\emptyapp>ruby script\rails runner -e production "p $LOADED_FEATURES.size" 772 When benchmark startup using w32time [8]: V:\emptyapp>timer ruby script\rails runner -e production "0" real 7.285 system 4.539 user 2.683 And patched Ruby: V:\emptyapp>timer ruby script\rails runner -e production "0" real 2.620 system 0.873 user 1.700 (best result taken from several warm ups). Now, a mid-sized application like Enki [9] which loads 1146 files in production mode, result in: V:\enki>timer ruby script\rails runner -e production "p $LOADED_FEATURES.size" 1146 real 22.620 system 11.497 user 11.076 Almost ((*23 seconds*)), compared to patched version: V:\enki>timer ruby script\rails runner -e production "0" real 11.013 system 1.981 user 8.938 This change also improves performance of (({rake})) inside Rails, from: V:\enki>timer rake -T ... real 8.689 system 0.015 user 0.000 To: V:\enki>timer rake -T ... real 3.307 system 0.000 user 0.031 Making normal operations more accessible. Looking forward for your thoughts on these changes. Thank you. [1] http://www.ruby-lang.org/en/news/2008/03/03/webric... [2] https://github.com/ruby/ruby/blob/trunk/lib/webric... [3] https://github.com/ruby/ruby/blob/trunk/lib/webric... [4] http://msdn.microsoft.com/en-us/library/windows/de... [5] https://github.com/luislavena/fenix [6] http://thecodeshop.github.com/ [7] https://github.com/jonforums/measurements [8] https://github.com/thecodeshop/w32time [9] https://github.com/xaviershay/enki =end
on 2012-08-04 23:55
on 2012-08-06 05:30
Issue #6836 has been updated by usa (Usaku NAKAMURA). First, I think this is the great job! This patch is very big, so I've not checked whole yet. I found that this includes a patch to WEBrick. So, I guess that this means there is an incompatibility in File.expand_path, doesn't it? What is the incompatibility? ---------------------------------------- Bug #6836: Improve File.expand_path performance in Windows https://bugs.ruby-lang.org/issues/6836#change-28669 Author: luislavena (Luis Lavena) Status: Assigned Priority: Normal Assignee: nobu (Nobuyoshi Nakada) Category: core Target version: 2.0.0 ruby -v: ruby 2.0.0dev (2012-08-04 trunk 36616) [i386-mingw32] =begin (original write up in markdown here: https://gist.github.com/3242245) == Background While investigating the performance issues of (({File.expand_path})) on Windows, Usaku Nakamura and Nobuyoshi Nakada on [ruby-core:39504] pointed out that due security concerns, accessing files on Windows required normalized paths. This was covered in the security update of March 2008, WEBrick file-access vulnerability [1]. After closer inspection of WEBrick code (mentioned by the security update), I noticed it uses (({File.expand_path})) to perform the heavy lifting of path normalization in the request. The code around this can be inspected in (({prevent_directory_traversal}))[2] and (({shift_path_info}))[3] methods. This approach performs a hit into the filesystem, contrary to its implementation in any other operating system. (({File.expand_path})) is heavily used by (({require})), which result in slow application startup, depending on the application size or number of gems it depends on. Stepping back for a second, we can see that the security issue is around WEBrick and the way it determines (({path_info})) absoluteness. It is also clear that to solve WEBrick security issue, a tax has been applied to the entire Ruby ecosystem, penalizing startup performance. With Hiroshi Shirosaki's help, we worked on a patch that: * Limit filesystem hit only to WEBrick, using Windows' GetLongPathName [4]. * Use a Windows-specific API to normalize paths * Improve encoding support. What started as an experiment named Fenix [5] has shown great results on a variety of systems. This patch has been integrated in TheCodeShop [6] releases of Ruby 1.9.3 and tested by Ruby-Core developers Hiroshi Shirosaki and myself. == Performance To demonstrate the benefits of this patch, I've used measurements [7] project and both (({core_require_empty})) and (({core_require_nested})) workloads, obtaining the following results: ruby 2.0.0dev (2012-08-03 trunk 36611) [i386-mingw32] Rehearsal ------------------------------------------------------ core_require_empty 1.186000 3.385000 4.571000 ( 4.676267) --------------------------------------------- total: 4.571000sec user system total real core_require_empty 1.217000 3.385000 4.602000 ( 4.643266) Rehearsal ------------------------------------------------------- core_require_nested 1.514000 3.760000 5.274000 ( 5.305303) ---------------------------------------------- total: 5.274000sec user system total real core_require_nested 1.466000 3.713000 5.179000 ( 5.233300) And with patch applied: ruby 2.0.0dev (2012-08-03 trunk 36611) [i386-mingw32] Rehearsal ------------------------------------------------------ core_require_empty 0.765000 1.077000 1.842000 ( 1.887603) --------------------------------------------- total: 1.842000sec user system total real core_require_empty 0.717000 1.123000 1.840000 ( 1.887603) Rehearsal ------------------------------------------------------- core_require_nested 0.717000 1.670000 2.387000 ( 2.480405) ---------------------------------------------- total: 2.387000sec user system total real core_require_nested 0.890000 1.528000 2.418000 ( 2.496004) Benchmarks were performed all on the same hardware and OS: * CPU: Core 2 Duo T7500 @ 2.20Ghz * RAM: 4GB * HDD: 1.5GB RAMdisk (ImDisk) * OS: Windows 7 Ultimate x64 All tests associated (both File and WEBrick ones) pass. Additional tests that exercise specific aspects of new function were added. Patch has been tested also against: * Visual Studio build of Ruby * Ubuntu 12.04 * Mac OSX And the patch didn't affect either build or tests of it. === Real life impact: Rails The biggest Ruby project affected by this is Rails applications. An empty Rails application on startup requires more than 700 files from different gems: V:\emptyapp>ruby script\rails runner -e production "p $LOADED_FEATURES.size" 772 When benchmark startup using w32time [8]: V:\emptyapp>timer ruby script\rails runner -e production "0" real 7.285 system 4.539 user 2.683 And patched Ruby: V:\emptyapp>timer ruby script\rails runner -e production "0" real 2.620 system 0.873 user 1.700 (best result taken from several warm ups). Now, a mid-sized application like Enki [9] which loads 1146 files in production mode, result in: V:\enki>timer ruby script\rails runner -e production "p $LOADED_FEATURES.size" 1146 real 22.620 system 11.497 user 11.076 Almost ((*23 seconds*)), compared to patched version: V:\enki>timer ruby script\rails runner -e production "0" real 11.013 system 1.981 user 8.938 This change also improves performance of (({rake})) inside Rails, from: V:\enki>timer rake -T ... real 8.689 system 0.015 user 0.000 To: V:\enki>timer rake -T ... real 3.307 system 0.000 user 0.031 Making normal operations more accessible. Looking forward for your thoughts on these changes. Thank you. [1] http://www.ruby-lang.org/en/news/2008/03/03/webric... [2] https://github.com/ruby/ruby/blob/trunk/lib/webric... [3] https://github.com/ruby/ruby/blob/trunk/lib/webric... [4] http://msdn.microsoft.com/en-us/library/windows/de... [5] https://github.com/luislavena/fenix [6] http://thecodeshop.github.com/ [7] https://github.com/jonforums/measurements [8] https://github.com/thecodeshop/w32time [9] https://github.com/xaviershay/enki =end
on 2012-08-06 16:04
Issue #6836 has been updated by luislavena (Luis Lavena). usa (Usaku NAKAMURA) wrote: > I found that this includes a patch to WEBrick. > So, I guess that this means there is an incompatibility in File.expand_path, doesn't it? > What is the incompatibility? WEBrick relies on File.expand_path to resolve the traversal (by expanding) but also by expanding possible short names into long names. Since this patch no longer hits the filesystem to determine if the path is a real file and expand the shortname into longname, we moved it to WEBrick. From what I understand from WEBrick test, I'm still not 100% sure about that particular patch, specially since other tools like Rack handle path traversal security without touching the filesystem. Rack doesn't rely on File.expand_path at all for traversal checks: https://github.com/rack/rack/pull/373#issuecomment-4684709 And the code: https://github.com/rack/rack/blob/master/lib/rack/... Our decision to maintain WEBrick tests was under the assumption that the test was doing something other than what Rack is doing here. Because of that, we decided to only expand the shortnames in WEBrick. IMO think Rack approach is better, as it doesn't rely on File.expand_path at all, but I could be wrong. ---------------------------------------- Bug #6836: Improve File.expand_path performance in Windows https://bugs.ruby-lang.org/issues/6836#change-28679 Author: luislavena (Luis Lavena) Status: Assigned Priority: Normal Assignee: nobu (Nobuyoshi Nakada) Category: core Target version: 2.0.0 ruby -v: ruby 2.0.0dev (2012-08-04 trunk 36616) [i386-mingw32] =begin (original write up in markdown here: https://gist.github.com/3242245) == Background While investigating the performance issues of (({File.expand_path})) on Windows, Usaku Nakamura and Nobuyoshi Nakada on [ruby-core:39504] pointed out that due security concerns, accessing files on Windows required normalized paths. This was covered in the security update of March 2008, WEBrick file-access vulnerability [1]. After closer inspection of WEBrick code (mentioned by the security update), I noticed it uses (({File.expand_path})) to perform the heavy lifting of path normalization in the request. The code around this can be inspected in (({prevent_directory_traversal}))[2] and (({shift_path_info}))[3] methods. This approach performs a hit into the filesystem, contrary to its implementation in any other operating system. (({File.expand_path})) is heavily used by (({require})), which result in slow application startup, depending on the application size or number of gems it depends on. Stepping back for a second, we can see that the security issue is around WEBrick and the way it determines (({path_info})) absoluteness. It is also clear that to solve WEBrick security issue, a tax has been applied to the entire Ruby ecosystem, penalizing startup performance. With Hiroshi Shirosaki's help, we worked on a patch that: * Limit filesystem hit only to WEBrick, using Windows' GetLongPathName [4]. * Use a Windows-specific API to normalize paths * Improve encoding support. What started as an experiment named Fenix [5] has shown great results on a variety of systems. This patch has been integrated in TheCodeShop [6] releases of Ruby 1.9.3 and tested by Ruby-Core developers Hiroshi Shirosaki and myself. == Performance To demonstrate the benefits of this patch, I've used measurements [7] project and both (({core_require_empty})) and (({core_require_nested})) workloads, obtaining the following results: ruby 2.0.0dev (2012-08-03 trunk 36611) [i386-mingw32] Rehearsal ------------------------------------------------------ core_require_empty 1.186000 3.385000 4.571000 ( 4.676267) --------------------------------------------- total: 4.571000sec user system total real core_require_empty 1.217000 3.385000 4.602000 ( 4.643266) Rehearsal ------------------------------------------------------- core_require_nested 1.514000 3.760000 5.274000 ( 5.305303) ---------------------------------------------- total: 5.274000sec user system total real core_require_nested 1.466000 3.713000 5.179000 ( 5.233300) And with patch applied: ruby 2.0.0dev (2012-08-03 trunk 36611) [i386-mingw32] Rehearsal ------------------------------------------------------ core_require_empty 0.765000 1.077000 1.842000 ( 1.887603) --------------------------------------------- total: 1.842000sec user system total real core_require_empty 0.717000 1.123000 1.840000 ( 1.887603) Rehearsal ------------------------------------------------------- core_require_nested 0.717000 1.670000 2.387000 ( 2.480405) ---------------------------------------------- total: 2.387000sec user system total real core_require_nested 0.890000 1.528000 2.418000 ( 2.496004) Benchmarks were performed all on the same hardware and OS: * CPU: Core 2 Duo T7500 @ 2.20Ghz * RAM: 4GB * HDD: 1.5GB RAMdisk (ImDisk) * OS: Windows 7 Ultimate x64 All tests associated (both File and WEBrick ones) pass. Additional tests that exercise specific aspects of new function were added. Patch has been tested also against: * Visual Studio build of Ruby * Ubuntu 12.04 * Mac OSX And the patch didn't affect either build or tests of it. === Real life impact: Rails The biggest Ruby project affected by this is Rails applications. An empty Rails application on startup requires more than 700 files from different gems: V:\emptyapp>ruby script\rails runner -e production "p $LOADED_FEATURES.size" 772 When benchmark startup using w32time [8]: V:\emptyapp>timer ruby script\rails runner -e production "0" real 7.285 system 4.539 user 2.683 And patched Ruby: V:\emptyapp>timer ruby script\rails runner -e production "0" real 2.620 system 0.873 user 1.700 (best result taken from several warm ups). Now, a mid-sized application like Enki [9] which loads 1146 files in production mode, result in: V:\enki>timer ruby script\rails runner -e production "p $LOADED_FEATURES.size" 1146 real 22.620 system 11.497 user 11.076 Almost ((*23 seconds*)), compared to patched version: V:\enki>timer ruby script\rails runner -e production "0" real 11.013 system 1.981 user 8.938 This change also improves performance of (({rake})) inside Rails, from: V:\enki>timer rake -T ... real 8.689 system 0.015 user 0.000 To: V:\enki>timer rake -T ... real 3.307 system 0.000 user 0.031 Making normal operations more accessible. Looking forward for your thoughts on these changes. Thank you. [1] http://www.ruby-lang.org/en/news/2008/03/03/webric... [2] https://github.com/ruby/ruby/blob/trunk/lib/webric... [3] https://github.com/ruby/ruby/blob/trunk/lib/webric... [4] http://msdn.microsoft.com/en-us/library/windows/de... [5] https://github.com/luislavena/fenix [6] http://thecodeshop.github.com/ [7] https://github.com/jonforums/measurements [8] https://github.com/thecodeshop/w32time [9] https://github.com/xaviershay/enki =end
on 2012-08-07 02:35
Hello,
Thank you for the explanation, Luis.
In message "[ruby-core:47021] [ruby-trunk - Bug #6836] Improve
File.expand_path performance in Windows"
on Aug.06,2012 23:02:11, <luislavena@gmail.com> wrote:
> Since this patch no longer hits the filesystem to determine if the path is a
real file and expand the shortname into longname, we moved it to WEBrick.
I see.
> Our decision to maintain WEBrick tests was under the assumption that the test
was doing something other than what Rack is doing here.
>
> Because of that, we decided to only expand the shortnames in WEBrick.
I understand, probably :)
> IMO think Rack approach is better, as it doesn't rely on File.expand_path at
all, but I could be wrong.
Normally we should not depend on the path normalization function
of File.expand_path.
Each program should perform peculiar safing processing which each
needs.
Therefore, I think that the approach of Rack may be better, too.
However, it is very difficult to write safe code because there
are too many traps in the file system of Windows.
It's impossible for non-Windows programmers in particular.
For instance, your WEBrick patch calls Win32 API with DL,
but Unix programmers will not know Win32 API.
Therefore, we made decision of pushing all the troubles in
File.expand_path.
We expected to help to write safe code in almost all cases.
Possibly this was not a good message.
Much program depending on File.expand_path may have been made
in the world.
It's the reason why I am very cowardly to change the behavior
of File.expand_path.
But I am not against to this patch.
I hope another people's review based on the above viewpoint.
Regards,
on 2012-08-07 03:03
On Mon, Aug 6, 2012 at 9:34 PM, U.Nakamura <usa@garbagecollect.jp> wrote: > Perhaps WEBrick patch can be changed to follow Rack approach and we remove DL altogether. > We expected to help to write safe code in almost all cases. > I understand, but also I noticed that File.expand_path is called all over the place for things like require, loading files and ensuring $LOAD_PATH is expanded (which was already expanded in the previous require). An empty Rails application generates 130K hits to File.expand_path function, but not caused by Rails itself, but instead require uses it internally. That alone accounts for all the performance brief users of Windows complain most of the time. > Possibly this was not a good message. > Much program depending on File.expand_path may have been made > in the world. > It's the reason why I am very cowardly to change the behavior > of File.expand_path. > > > But I am not against to this patch. > I hope another people's review based on the above viewpoint. > I started working on this patch August 2011, almost a year ago. Hiroshi and myself added the needed changes to make it work across different encoding and versions of Windows. Worth to mention this has been out in the wild since January, and we had a successful user-base of people using it thanks to TheCodeShop binary releases. While I was thinking we could consider backport this to Ruby 1.9.3 (similar to improved IO), but maybe is time that Ruby 2.0 corrects those assumptions. Perhaps you can ask for feedback to the Japanese developers that use Windows to test it out. I can definitely workout binary packages with the patch applied if they don't want to compile themselves. The more eyes into this will help us stop any remaining issue. Once again, thank you for taking the time to look into this and much appreciated your thoughts.
on 2012-08-07 07:47
Issue #6836 has been updated by h.shirosaki (Hiroshi Shirosaki). Other web servers on Windows also have Windows Short (8.3) Filenames security issue. http://www.coresecurity.com/content/filename-pseud... http://www.acunetix.com/blog/web-security-zone/art... If a user needs to care of security for short name, perhaps disabling Windows 8.3 short name creation would be a possible solution? Expanding short name to long name is expensive and using it frequently causes big performance difference between Windows and Unix. I think this change in 'require' and 'load' will be useful for most Windows users since not a few Windows users complain about slowness. ---------------------------------------- Bug #6836: Improve File.expand_path performance in Windows https://bugs.ruby-lang.org/issues/6836#change-28700 Author: luislavena (Luis Lavena) Status: Assigned Priority: Normal Assignee: nobu (Nobuyoshi Nakada) Category: core Target version: 2.0.0 ruby -v: ruby 2.0.0dev (2012-08-04 trunk 36616) [i386-mingw32] =begin (original write up in markdown here: https://gist.github.com/3242245) == Background While investigating the performance issues of (({File.expand_path})) on Windows, Usaku Nakamura and Nobuyoshi Nakada on [ruby-core:39504] pointed out that due security concerns, accessing files on Windows required normalized paths. This was covered in the security update of March 2008, WEBrick file-access vulnerability [1]. After closer inspection of WEBrick code (mentioned by the security update), I noticed it uses (({File.expand_path})) to perform the heavy lifting of path normalization in the request. The code around this can be inspected in (({prevent_directory_traversal}))[2] and (({shift_path_info}))[3] methods. This approach performs a hit into the filesystem, contrary to its implementation in any other operating system. (({File.expand_path})) is heavily used by (({require})), which result in slow application startup, depending on the application size or number of gems it depends on. Stepping back for a second, we can see that the security issue is around WEBrick and the way it determines (({path_info})) absoluteness. It is also clear that to solve WEBrick security issue, a tax has been applied to the entire Ruby ecosystem, penalizing startup performance. With Hiroshi Shirosaki's help, we worked on a patch that: * Limit filesystem hit only to WEBrick, using Windows' GetLongPathName [4]. * Use a Windows-specific API to normalize paths * Improve encoding support. What started as an experiment named Fenix [5] has shown great results on a variety of systems. This patch has been integrated in TheCodeShop [6] releases of Ruby 1.9.3 and tested by Ruby-Core developers Hiroshi Shirosaki and myself. == Performance To demonstrate the benefits of this patch, I've used measurements [7] project and both (({core_require_empty})) and (({core_require_nested})) workloads, obtaining the following results: ruby 2.0.0dev (2012-08-03 trunk 36611) [i386-mingw32] Rehearsal ------------------------------------------------------ core_require_empty 1.186000 3.385000 4.571000 ( 4.676267) --------------------------------------------- total: 4.571000sec user system total real core_require_empty 1.217000 3.385000 4.602000 ( 4.643266) Rehearsal ------------------------------------------------------- core_require_nested 1.514000 3.760000 5.274000 ( 5.305303) ---------------------------------------------- total: 5.274000sec user system total real core_require_nested 1.466000 3.713000 5.179000 ( 5.233300) And with patch applied: ruby 2.0.0dev (2012-08-03 trunk 36611) [i386-mingw32] Rehearsal ------------------------------------------------------ core_require_empty 0.765000 1.077000 1.842000 ( 1.887603) --------------------------------------------- total: 1.842000sec user system total real core_require_empty 0.717000 1.123000 1.840000 ( 1.887603) Rehearsal ------------------------------------------------------- core_require_nested 0.717000 1.670000 2.387000 ( 2.480405) ---------------------------------------------- total: 2.387000sec user system total real core_require_nested 0.890000 1.528000 2.418000 ( 2.496004) Benchmarks were performed all on the same hardware and OS: * CPU: Core 2 Duo T7500 @ 2.20Ghz * RAM: 4GB * HDD: 1.5GB RAMdisk (ImDisk) * OS: Windows 7 Ultimate x64 All tests associated (both File and WEBrick ones) pass. Additional tests that exercise specific aspects of new function were added. Patch has been tested also against: * Visual Studio build of Ruby * Ubuntu 12.04 * Mac OSX And the patch didn't affect either build or tests of it. === Real life impact: Rails The biggest Ruby project affected by this is Rails applications. An empty Rails application on startup requires more than 700 files from different gems: V:\emptyapp>ruby script\rails runner -e production "p $LOADED_FEATURES.size" 772 When benchmark startup using w32time [8]: V:\emptyapp>timer ruby script\rails runner -e production "0" real 7.285 system 4.539 user 2.683 And patched Ruby: V:\emptyapp>timer ruby script\rails runner -e production "0" real 2.620 system 0.873 user 1.700 (best result taken from several warm ups). Now, a mid-sized application like Enki [9] which loads 1146 files in production mode, result in: V:\enki>timer ruby script\rails runner -e production "p $LOADED_FEATURES.size" 1146 real 22.620 system 11.497 user 11.076 Almost ((*23 seconds*)), compared to patched version: V:\enki>timer ruby script\rails runner -e production "0" real 11.013 system 1.981 user 8.938 This change also improves performance of (({rake})) inside Rails, from: V:\enki>timer rake -T ... real 8.689 system 0.015 user 0.000 To: V:\enki>timer rake -T ... real 3.307 system 0.000 user 0.031 Making normal operations more accessible. Looking forward for your thoughts on these changes. Thank you. [1] http://www.ruby-lang.org/en/news/2008/03/03/webric... [2] https://github.com/ruby/ruby/blob/trunk/lib/webric... [3] https://github.com/ruby/ruby/blob/trunk/lib/webric... [4] http://msdn.microsoft.com/en-us/library/windows/de... [5] https://github.com/luislavena/fenix [6] http://thecodeshop.github.com/ [7] https://github.com/jonforums/measurements [8] https://github.com/thecodeshop/w32time [9] https://github.com/xaviershay/enki =end
on 2012-08-07 09:28
Hello,
In message "[ruby-core:47045] [ruby-trunk - Bug #6836] Improve
File.expand_path performance in Windows"
on Aug.07,2012 14:46:15, <h.shirosaki@gmail.com> wrote:
> Expanding short name to long name is expensive and using it frequently causes
big performance difference between Windows and Unix. I think this change in
'require' and 'load' will be useful for most Windows users since not a few Windows
users complain about slowness.
The Primary Principle: Security is not exchangeable for performance.
the secondary principle: but if the software is not usable by low
performance, security looses its meaning.
If the performance problem is in 'require' and 'load', change only them
and be stayed File.expand_path the same behavior.
Can't do so?
Regards,
on 2012-08-07 13:28
On Tue, Aug 7, 2012 at 2:34 AM, U.Nakamura <usa@garbagecollect.jp> wrote: > However, it is very difficult to write safe code because there > are too many traps in the file system of Windows. > It's impossible for non-Windows programmers in particular. > For instance, your WEBrick patch calls Win32 API with DL, > but Unix programmers will not know Win32 API. > Therefore, we made decision of pushing all the troubles in > File.expand_path. > We expected to help to write safe code in almost all cases. Having File.real_path instead would be nice.
on 2012-08-07 15:55
On Tue, Aug 7, 2012 at 4:28 AM, U.Nakamura <usa@garbagecollect.jp> wrote: > I agree with you, trading security over performance or viceversa is not good. > > If the performance problem is in 'require' and 'load', change only them > and be stayed File.expand_path the same behavior. > Can't do so? > Problem is expand_path is used all over the place: load.c: rb_get_expanded_load_path rb_feature_provided rb_feature_p file.c: rb_file_absolute_path rb_file_identical_p rb_file_s_absolute_path rb_find_file_ext_safe rb_find_file_safe ruby.c: expand_include_path Changing all those places seems more complex than just changing WEBrick. Perhaps we can make File.realpath solve that for us, after all, realpath is supposed to resolve symlinks and perhaps could be used to expand shortnames into longnames. From my point of view: core security is important, so is performance. WEBrick is stdlib, not core, changes to make stdlib happy should not compromise core security or performance. If shortnames (outside web) was considered a security issue for core then I would agree that expanding shortnames needs to be in core. I can start working on making realpath use newer Windows API (instead of stat) to obtain the expanded filename. Perhaps that will be a nice alternative for WEBrick.
on 2012-08-08 10:33
Issue #6836 has been updated by h.shirosaki (Hiroshi Shirosaki). usa (Usaku NAKAMURA) wrote: > If the performance problem is in 'require' and 'load', change only them > and be stayed File.expand_path the same behavior. > Can't do so? > I think that might be possible, though changing that properly seems not easy. I created an experimental patch for that. This patch passed webrick test and became faster. https://gist.github.com/3293339 Changed functions: load.c: rb_get_expanded_load_path rb_feature_provided rb_feature_p file.c: rb_find_file_ext_safe rb_find_file_safe ---------------------------------------- Bug #6836: Improve File.expand_path performance in Windows https://bugs.ruby-lang.org/issues/6836#change-28723 Author: luislavena (Luis Lavena) Status: Assigned Priority: Normal Assignee: nobu (Nobuyoshi Nakada) Category: core Target version: 2.0.0 ruby -v: ruby 2.0.0dev (2012-08-04 trunk 36616) [i386-mingw32] =begin (original write up in markdown here: https://gist.github.com/3242245) == Background While investigating the performance issues of (({File.expand_path})) on Windows, Usaku Nakamura and Nobuyoshi Nakada on [ruby-core:39504] pointed out that due security concerns, accessing files on Windows required normalized paths. This was covered in the security update of March 2008, WEBrick file-access vulnerability [1]. After closer inspection of WEBrick code (mentioned by the security update), I noticed it uses (({File.expand_path})) to perform the heavy lifting of path normalization in the request. The code around this can be inspected in (({prevent_directory_traversal}))[2] and (({shift_path_info}))[3] methods. This approach performs a hit into the filesystem, contrary to its implementation in any other operating system. (({File.expand_path})) is heavily used by (({require})), which result in slow application startup, depending on the application size or number of gems it depends on. Stepping back for a second, we can see that the security issue is around WEBrick and the way it determines (({path_info})) absoluteness. It is also clear that to solve WEBrick security issue, a tax has been applied to the entire Ruby ecosystem, penalizing startup performance. With Hiroshi Shirosaki's help, we worked on a patch that: * Limit filesystem hit only to WEBrick, using Windows' GetLongPathName [4]. * Use a Windows-specific API to normalize paths * Improve encoding support. What started as an experiment named Fenix [5] has shown great results on a variety of systems. This patch has been integrated in TheCodeShop [6] releases of Ruby 1.9.3 and tested by Ruby-Core developers Hiroshi Shirosaki and myself. == Performance To demonstrate the benefits of this patch, I've used measurements [7] project and both (({core_require_empty})) and (({core_require_nested})) workloads, obtaining the following results: ruby 2.0.0dev (2012-08-03 trunk 36611) [i386-mingw32] Rehearsal ------------------------------------------------------ core_require_empty 1.186000 3.385000 4.571000 ( 4.676267) --------------------------------------------- total: 4.571000sec user system total real core_require_empty 1.217000 3.385000 4.602000 ( 4.643266) Rehearsal ------------------------------------------------------- core_require_nested 1.514000 3.760000 5.274000 ( 5.305303) ---------------------------------------------- total: 5.274000sec user system total real core_require_nested 1.466000 3.713000 5.179000 ( 5.233300) And with patch applied: ruby 2.0.0dev (2012-08-03 trunk 36611) [i386-mingw32] Rehearsal ------------------------------------------------------ core_require_empty 0.765000 1.077000 1.842000 ( 1.887603) --------------------------------------------- total: 1.842000sec user system total real core_require_empty 0.717000 1.123000 1.840000 ( 1.887603) Rehearsal ------------------------------------------------------- core_require_nested 0.717000 1.670000 2.387000 ( 2.480405) ---------------------------------------------- total: 2.387000sec user system total real core_require_nested 0.890000 1.528000 2.418000 ( 2.496004) Benchmarks were performed all on the same hardware and OS: * CPU: Core 2 Duo T7500 @ 2.20Ghz * RAM: 4GB * HDD: 1.5GB RAMdisk (ImDisk) * OS: Windows 7 Ultimate x64 All tests associated (both File and WEBrick ones) pass. Additional tests that exercise specific aspects of new function were added. Patch has been tested also against: * Visual Studio build of Ruby * Ubuntu 12.04 * Mac OSX And the patch didn't affect either build or tests of it. === Real life impact: Rails The biggest Ruby project affected by this is Rails applications. An empty Rails application on startup requires more than 700 files from different gems: V:\emptyapp>ruby script\rails runner -e production "p $LOADED_FEATURES.size" 772 When benchmark startup using w32time [8]: V:\emptyapp>timer ruby script\rails runner -e production "0" real 7.285 system 4.539 user 2.683 And patched Ruby: V:\emptyapp>timer ruby script\rails runner -e production "0" real 2.620 system 0.873 user 1.700 (best result taken from several warm ups). Now, a mid-sized application like Enki [9] which loads 1146 files in production mode, result in: V:\enki>timer ruby script\rails runner -e production "p $LOADED_FEATURES.size" 1146 real 22.620 system 11.497 user 11.076 Almost ((*23 seconds*)), compared to patched version: V:\enki>timer ruby script\rails runner -e production "0" real 11.013 system 1.981 user 8.938 This change also improves performance of (({rake})) inside Rails, from: V:\enki>timer rake -T ... real 8.689 system 0.015 user 0.000 To: V:\enki>timer rake -T ... real 3.307 system 0.000 user 0.031 Making normal operations more accessible. Looking forward for your thoughts on these changes. Thank you. [1] http://www.ruby-lang.org/en/news/2008/03/03/webric... [2] https://github.com/ruby/ruby/blob/trunk/lib/webric... [3] https://github.com/ruby/ruby/blob/trunk/lib/webric... [4] http://msdn.microsoft.com/en-us/library/windows/de... [5] https://github.com/luislavena/fenix [6] http://thecodeshop.github.com/ [7] https://github.com/jonforums/measurements [8] https://github.com/thecodeshop/w32time [9] https://github.com/xaviershay/enki =end
on 2012-08-08 14:42
Issue #6836 has been updated by luislavena (Luis Lavena). h.shirosaki (Hiroshi Shirosaki) wrote: > Hiroshi, how that compares with the original patch? Thank you. ---------------------------------------- Bug #6836: Improve File.expand_path performance in Windows https://bugs.ruby-lang.org/issues/6836#change-28728 Author: luislavena (Luis Lavena) Status: Assigned Priority: Normal Assignee: nobu (Nobuyoshi Nakada) Category: core Target version: 2.0.0 ruby -v: ruby 2.0.0dev (2012-08-04 trunk 36616) [i386-mingw32] =begin (original write up in markdown here: https://gist.github.com/3242245) == Background While investigating the performance issues of (({File.expand_path})) on Windows, Usaku Nakamura and Nobuyoshi Nakada on [ruby-core:39504] pointed out that due security concerns, accessing files on Windows required normalized paths. This was covered in the security update of March 2008, WEBrick file-access vulnerability [1]. After closer inspection of WEBrick code (mentioned by the security update), I noticed it uses (({File.expand_path})) to perform the heavy lifting of path normalization in the request. The code around this can be inspected in (({prevent_directory_traversal}))[2] and (({shift_path_info}))[3] methods. This approach performs a hit into the filesystem, contrary to its implementation in any other operating system. (({File.expand_path})) is heavily used by (({require})), which result in slow application startup, depending on the application size or number of gems it depends on. Stepping back for a second, we can see that the security issue is around WEBrick and the way it determines (({path_info})) absoluteness. It is also clear that to solve WEBrick security issue, a tax has been applied to the entire Ruby ecosystem, penalizing startup performance. With Hiroshi Shirosaki's help, we worked on a patch that: * Limit filesystem hit only to WEBrick, using Windows' GetLongPathName [4]. * Use a Windows-specific API to normalize paths * Improve encoding support. What started as an experiment named Fenix [5] has shown great results on a variety of systems. This patch has been integrated in TheCodeShop [6] releases of Ruby 1.9.3 and tested by Ruby-Core developers Hiroshi Shirosaki and myself. == Performance To demonstrate the benefits of this patch, I've used measurements [7] project and both (({core_require_empty})) and (({core_require_nested})) workloads, obtaining the following results: ruby 2.0.0dev (2012-08-03 trunk 36611) [i386-mingw32] Rehearsal ------------------------------------------------------ core_require_empty 1.186000 3.385000 4.571000 ( 4.676267) --------------------------------------------- total: 4.571000sec user system total real core_require_empty 1.217000 3.385000 4.602000 ( 4.643266) Rehearsal ------------------------------------------------------- core_require_nested 1.514000 3.760000 5.274000 ( 5.305303) ---------------------------------------------- total: 5.274000sec user system total real core_require_nested 1.466000 3.713000 5.179000 ( 5.233300) And with patch applied: ruby 2.0.0dev (2012-08-03 trunk 36611) [i386-mingw32] Rehearsal ------------------------------------------------------ core_require_empty 0.765000 1.077000 1.842000 ( 1.887603) --------------------------------------------- total: 1.842000sec user system total real core_require_empty 0.717000 1.123000 1.840000 ( 1.887603) Rehearsal ------------------------------------------------------- core_require_nested 0.717000 1.670000 2.387000 ( 2.480405) ---------------------------------------------- total: 2.387000sec user system total real core_require_nested 0.890000 1.528000 2.418000 ( 2.496004) Benchmarks were performed all on the same hardware and OS: * CPU: Core 2 Duo T7500 @ 2.20Ghz * RAM: 4GB * HDD: 1.5GB RAMdisk (ImDisk) * OS: Windows 7 Ultimate x64 All tests associated (both File and WEBrick ones) pass. Additional tests that exercise specific aspects of new function were added. Patch has been tested also against: * Visual Studio build of Ruby * Ubuntu 12.04 * Mac OSX And the patch didn't affect either build or tests of it. === Real life impact: Rails The biggest Ruby project affected by this is Rails applications. An empty Rails application on startup requires more than 700 files from different gems: V:\emptyapp>ruby script\rails runner -e production "p $LOADED_FEATURES.size" 772 When benchmark startup using w32time [8]: V:\emptyapp>timer ruby script\rails runner -e production "0" real 7.285 system 4.539 user 2.683 And patched Ruby: V:\emptyapp>timer ruby script\rails runner -e production "0" real 2.620 system 0.873 user 1.700 (best result taken from several warm ups). Now, a mid-sized application like Enki [9] which loads 1146 files in production mode, result in: V:\enki>timer ruby script\rails runner -e production "p $LOADED_FEATURES.size" 1146 real 22.620 system 11.497 user 11.076 Almost ((*23 seconds*)), compared to patched version: V:\enki>timer ruby script\rails runner -e production "0" real 11.013 system 1.981 user 8.938 This change also improves performance of (({rake})) inside Rails, from: V:\enki>timer rake -T ... real 8.689 system 0.015 user 0.000 To: V:\enki>timer rake -T ... real 3.307 system 0.000 user 0.031 Making normal operations more accessible. Looking forward for your thoughts on these changes. Thank you. [1] http://www.ruby-lang.org/en/news/2008/03/03/webric... [2] https://github.com/ruby/ruby/blob/trunk/lib/webric... [3] https://github.com/ruby/ruby/blob/trunk/lib/webric... [4] http://msdn.microsoft.com/en-us/library/windows/de... [5] https://github.com/luislavena/fenix [6] http://thecodeshop.github.com/ [7] https://github.com/jonforums/measurements [8] https://github.com/thecodeshop/w32time [9] https://github.com/xaviershay/enki =end
on 2012-08-09 00:38
On Wed, Aug 8, 2012 at 9:42 PM, luislavena (Luis Lavena) <luislavena@gmail.com> wrote: >> >> I think that might be possible, though changing that properly seems not easy. >> I created an experimental patch for that. This patch passed webrick test and became faster. >> >> https://gist.github.com/3293339 >> > > Hiroshi, how that compares with the original patch? > Above patch is small and easy to review. We can change the original patch to follow the above approach. We have code for short name expansion. I understand the original win32/file.c patch has another merit. We can write windows specific code without too many #ifdef macro which are hard to read.
on 2012-08-27 14:01
Issue #6836 has been updated by luislavena (Luis Lavena).
File improve-require-and-file-expand_path-windows.v2.diff added
=begin
usa (Usaku NAKAMURA) wrote:
> Hello,
>
> If the performance problem is in 'require' and 'load', change only
them
> and be stayed File.expand_path the same behavior.
> Can't do so?
Thank you Usa for your feedback.
Based on Hiroshi experiment I've worked in an updated patch (attached)
This new patch shows the same performance boost on (({require})) without
breaking backward compatibility of (({File.expand_path}))
trunk:
ruby 2.0.0dev (2012-08-23 trunk 36786) [i386-mingw32]
Rehearsal ------------------------------------------------------
core_require_empty 1.264000 3.151000 4.415000 ( 4.446254)
--------------------------------------------- total: 4.415000sec
user system total real
core_require_empty 1.154000 3.229000 4.383000 ( 4.432253)
Rehearsal -------------------------------------------------------
core_require_nested 1.248000 3.447000 4.695000 ( 4.707269)
---------------------------------------------- total: 4.695000sec
user system total real
core_require_nested 1.467000 3.214000 4.681000 ( 4.699268)
patched:
ruby 2.0.0dev (2012-08-23 trunk 36786) [i386-mingw32]
Rehearsal ------------------------------------------------------
core_require_empty 0.593000 0.936000 1.529000 ( 1.595091)
--------------------------------------------- total: 1.529000sec
user system total real
core_require_empty 0.624000 0.936000 1.560000 ( 1.577090)
ruby 2.0.0dev (2012-08-23 trunk 36786) [i386-mingw32]
Rehearsal -------------------------------------------------------
core_require_nested 0.843000 0.998000 1.841000 ( 1.855106)
---------------------------------------------- total: 1.841000sec
user system total real
core_require_nested 0.764000 1.061000 1.825000 ( 1.838106)
This also improves Rails and Rake startup time
From (trunk, mid-size Rails app):
V:\enki>timer ruby script\rails runner -e production "p
$LOADED_FEATURES.size"
1135
real 22.710
system 11.013
user 11.575
V:\enki>timer rake -T
...
real 8.868
system 0.031
user 0.000
To (patched):
V:\enki>timer ruby script\rails runner -e production "0"
real 10.735
system 1.716
user 8.923
V:\enki>timer rake -T
...
real 3.068
system 0.015
user 0.015
Updated the Gist that contains the benchmark and patch:
https://gist.github.com/3242245
Considering the size of the patch and to make it more easy to review,
I've pushed to my fork on GitHub the individual commits and explanation
of each change here:
https://github.com/luislavena/ruby/compare/improve...
Looking forward for your comments and feedback.
Thank you for your time.
=end
----------------------------------------
Bug #6836: Improve File.expand_path performance in Windows
https://bugs.ruby-lang.org/issues/6836#change-28999
Author: luislavena (Luis Lavena)
Status: Assigned
Priority: Normal
Assignee: nobu (Nobuyoshi Nakada)
Category: core
Target version: 2.0.0
ruby -v: ruby 2.0.0dev (2012-08-04 trunk 36616) [i386-mingw32]
=begin
(original write up in markdown here: https://gist.github.com/3242245)
== Background
While investigating the performance issues of (({File.expand_path})) on
Windows,
Usaku Nakamura and Nobuyoshi Nakada on [ruby-core:39504] pointed out
that
due security concerns, accessing files on Windows required normalized
paths.
This was covered in the security update of March 2008, WEBrick
file-access
vulnerability [1].
After closer inspection of WEBrick code (mentioned by the security
update),
I noticed it uses (({File.expand_path})) to perform the heavy lifting of
path
normalization in the request.
The code around this can be inspected in
(({prevent_directory_traversal}))[2]
and (({shift_path_info}))[3] methods.
This approach performs a hit into the filesystem, contrary to its
implementation in any other operating system.
(({File.expand_path})) is heavily used by (({require})), which result in
slow
application startup, depending on the application size or number of gems
it
depends on.
Stepping back for a second, we can see that the security issue is around
WEBrick and the way it determines (({path_info})) absoluteness.
It is also clear that to solve WEBrick security issue, a tax has been
applied
to the entire Ruby ecosystem, penalizing startup performance.
With Hiroshi Shirosaki's help, we worked on a patch that:
* Limit filesystem hit only to WEBrick, using Windows' GetLongPathName
[4].
* Use a Windows-specific API to normalize paths
* Improve encoding support.
What started as an experiment named Fenix [5] has shown great results on
a
variety of systems.
This patch has been integrated in TheCodeShop [6] releases of Ruby 1.9.3
and
tested by Ruby-Core developers Hiroshi Shirosaki and myself.
== Performance
To demonstrate the benefits of this patch, I've used measurements [7]
project
and both (({core_require_empty})) and (({core_require_nested}))
workloads,
obtaining the following results:
ruby 2.0.0dev (2012-08-03 trunk 36611) [i386-mingw32]
Rehearsal ------------------------------------------------------
core_require_empty 1.186000 3.385000 4.571000 ( 4.676267)
--------------------------------------------- total: 4.571000sec
user system total real
core_require_empty 1.217000 3.385000 4.602000 ( 4.643266)
Rehearsal -------------------------------------------------------
core_require_nested 1.514000 3.760000 5.274000 ( 5.305303)
---------------------------------------------- total: 5.274000sec
user system total real
core_require_nested 1.466000 3.713000 5.179000 ( 5.233300)
And with patch applied:
ruby 2.0.0dev (2012-08-03 trunk 36611) [i386-mingw32]
Rehearsal ------------------------------------------------------
core_require_empty 0.765000 1.077000 1.842000 ( 1.887603)
--------------------------------------------- total: 1.842000sec
user system total real
core_require_empty 0.717000 1.123000 1.840000 ( 1.887603)
Rehearsal -------------------------------------------------------
core_require_nested 0.717000 1.670000 2.387000 ( 2.480405)
---------------------------------------------- total: 2.387000sec
user system total real
core_require_nested 0.890000 1.528000 2.418000 ( 2.496004)
Benchmarks were performed all on the same hardware and OS:
* CPU: Core 2 Duo T7500 @ 2.20Ghz
* RAM: 4GB
* HDD: 1.5GB RAMdisk (ImDisk)
* OS: Windows 7 Ultimate x64
All tests associated (both File and WEBrick ones) pass.
Additional tests that exercise specific aspects of new function were
added.
Patch has been tested also against:
* Visual Studio build of Ruby
* Ubuntu 12.04
* Mac OSX
And the patch didn't affect either build or tests of it.
=== Real life impact: Rails
The biggest Ruby project affected by this is Rails applications.
An empty Rails application on startup requires more than 700 files from
different gems:
V:\emptyapp>ruby script\rails runner -e production "p
$LOADED_FEATURES.size"
772
When benchmark startup using w32time [8]:
V:\emptyapp>timer ruby script\rails runner -e production "0"
real 7.285
system 4.539
user 2.683
And patched Ruby:
V:\emptyapp>timer ruby script\rails runner -e production "0"
real 2.620
system 0.873
user 1.700
(best result taken from several warm ups).
Now, a mid-sized application like Enki [9] which loads 1146 files in
production mode, result in:
V:\enki>timer ruby script\rails runner -e production "p
$LOADED_FEATURES.size"
1146
real 22.620
system 11.497
user 11.076
Almost ((*23 seconds*)), compared to patched version:
V:\enki>timer ruby script\rails runner -e production "0"
real 11.013
system 1.981
user 8.938
This change also improves performance of (({rake})) inside Rails, from:
V:\enki>timer rake -T
...
real 8.689
system 0.015
user 0.000
To:
V:\enki>timer rake -T
...
real 3.307
system 0.000
user 0.031
Making normal operations more accessible.
Looking forward for your thoughts on these changes.
Thank you.
[1]
http://www.ruby-lang.org/en/news/2008/03/03/webric...
[2]
https://github.com/ruby/ruby/blob/trunk/lib/webric...
[3]
https://github.com/ruby/ruby/blob/trunk/lib/webric...
[4]
http://msdn.microsoft.com/en-us/library/windows/de...
[5] https://github.com/luislavena/fenix
[6] http://thecodeshop.github.com/
[7] https://github.com/jonforums/measurements
[8] https://github.com/thecodeshop/w32time
[9] https://github.com/xaviershay/enki
=end
on 2012-08-27 14:01
Issue #6836 has been updated by luislavena (Luis Lavena). Assignee changed from nobu (Nobuyoshi Nakada) to usa (Usaku NAKAMURA) ---------------------------------------- Bug #6836: Improve File.expand_path performance in Windows https://bugs.ruby-lang.org/issues/6836#change-29000 Author: luislavena (Luis Lavena) Status: Assigned Priority: Normal Assignee: usa (Usaku NAKAMURA) Category: core Target version: 2.0.0 ruby -v: ruby 2.0.0dev (2012-08-04 trunk 36616) [i386-mingw32] =begin (original write up in markdown here: https://gist.github.com/3242245) == Background While investigating the performance issues of (({File.expand_path})) on Windows, Usaku Nakamura and Nobuyoshi Nakada on [ruby-core:39504] pointed out that due security concerns, accessing files on Windows required normalized paths. This was covered in the security update of March 2008, WEBrick file-access vulnerability [1]. After closer inspection of WEBrick code (mentioned by the security update), I noticed it uses (({File.expand_path})) to perform the heavy lifting of path normalization in the request. The code around this can be inspected in (({prevent_directory_traversal}))[2] and (({shift_path_info}))[3] methods. This approach performs a hit into the filesystem, contrary to its implementation in any other operating system. (({File.expand_path})) is heavily used by (({require})), which result in slow application startup, depending on the application size or number of gems it depends on. Stepping back for a second, we can see that the security issue is around WEBrick and the way it determines (({path_info})) absoluteness. It is also clear that to solve WEBrick security issue, a tax has been applied to the entire Ruby ecosystem, penalizing startup performance. With Hiroshi Shirosaki's help, we worked on a patch that: * Limit filesystem hit only to WEBrick, using Windows' GetLongPathName [4]. * Use a Windows-specific API to normalize paths * Improve encoding support. What started as an experiment named Fenix [5] has shown great results on a variety of systems. This patch has been integrated in TheCodeShop [6] releases of Ruby 1.9.3 and tested by Ruby-Core developers Hiroshi Shirosaki and myself. == Performance To demonstrate the benefits of this patch, I've used measurements [7] project and both (({core_require_empty})) and (({core_require_nested})) workloads, obtaining the following results: ruby 2.0.0dev (2012-08-03 trunk 36611) [i386-mingw32] Rehearsal ------------------------------------------------------ core_require_empty 1.186000 3.385000 4.571000 ( 4.676267) --------------------------------------------- total: 4.571000sec user system total real core_require_empty 1.217000 3.385000 4.602000 ( 4.643266) Rehearsal ------------------------------------------------------- core_require_nested 1.514000 3.760000 5.274000 ( 5.305303) ---------------------------------------------- total: 5.274000sec user system total real core_require_nested 1.466000 3.713000 5.179000 ( 5.233300) And with patch applied: ruby 2.0.0dev (2012-08-03 trunk 36611) [i386-mingw32] Rehearsal ------------------------------------------------------ core_require_empty 0.765000 1.077000 1.842000 ( 1.887603) --------------------------------------------- total: 1.842000sec user system total real core_require_empty 0.717000 1.123000 1.840000 ( 1.887603) Rehearsal ------------------------------------------------------- core_require_nested 0.717000 1.670000 2.387000 ( 2.480405) ---------------------------------------------- total: 2.387000sec user system total real core_require_nested 0.890000 1.528000 2.418000 ( 2.496004) Benchmarks were performed all on the same hardware and OS: * CPU: Core 2 Duo T7500 @ 2.20Ghz * RAM: 4GB * HDD: 1.5GB RAMdisk (ImDisk) * OS: Windows 7 Ultimate x64 All tests associated (both File and WEBrick ones) pass. Additional tests that exercise specific aspects of new function were added. Patch has been tested also against: * Visual Studio build of Ruby * Ubuntu 12.04 * Mac OSX And the patch didn't affect either build or tests of it. === Real life impact: Rails The biggest Ruby project affected by this is Rails applications. An empty Rails application on startup requires more than 700 files from different gems: V:\emptyapp>ruby script\rails runner -e production "p $LOADED_FEATURES.size" 772 When benchmark startup using w32time [8]: V:\emptyapp>timer ruby script\rails runner -e production "0" real 7.285 system 4.539 user 2.683 And patched Ruby: V:\emptyapp>timer ruby script\rails runner -e production "0" real 2.620 system 0.873 user 1.700 (best result taken from several warm ups). Now, a mid-sized application like Enki [9] which loads 1146 files in production mode, result in: V:\enki>timer ruby script\rails runner -e production "p $LOADED_FEATURES.size" 1146 real 22.620 system 11.497 user 11.076 Almost ((*23 seconds*)), compared to patched version: V:\enki>timer ruby script\rails runner -e production "0" real 11.013 system 1.981 user 8.938 This change also improves performance of (({rake})) inside Rails, from: V:\enki>timer rake -T ... real 8.689 system 0.015 user 0.000 To: V:\enki>timer rake -T ... real 3.307 system 0.000 user 0.031 Making normal operations more accessible. Looking forward for your thoughts on these changes. Thank you. [1] http://www.ruby-lang.org/en/news/2008/03/03/webric... [2] https://github.com/ruby/ruby/blob/trunk/lib/webric... [3] https://github.com/ruby/ruby/blob/trunk/lib/webric... [4] http://msdn.microsoft.com/en-us/library/windows/de... [5] https://github.com/luislavena/fenix [6] http://thecodeshop.github.com/ [7] https://github.com/jonforums/measurements [8] https://github.com/thecodeshop/w32time [9] https://github.com/xaviershay/enki =end
on 2012-08-27 14:01
Issue #6836 has been updated by usa (Usaku NAKAMURA). Assignee changed from usa (Usaku NAKAMURA) to luislavena (Luis Lavena) Thanks your efforts. Please commit. Let's test all together. ---------------------------------------- Bug #6836: Improve File.expand_path performance in Windows https://bugs.ruby-lang.org/issues/6836#change-29003 Author: luislavena (Luis Lavena) Status: Assigned Priority: Normal Assignee: luislavena (Luis Lavena) Category: core Target version: 2.0.0 ruby -v: ruby 2.0.0dev (2012-08-04 trunk 36616) [i386-mingw32] =begin (original write up in markdown here: https://gist.github.com/3242245) == Background While investigating the performance issues of (({File.expand_path})) on Windows, Usaku Nakamura and Nobuyoshi Nakada on [ruby-core:39504] pointed out that due security concerns, accessing files on Windows required normalized paths. This was covered in the security update of March 2008, WEBrick file-access vulnerability [1]. After closer inspection of WEBrick code (mentioned by the security update), I noticed it uses (({File.expand_path})) to perform the heavy lifting of path normalization in the request. The code around this can be inspected in (({prevent_directory_traversal}))[2] and (({shift_path_info}))[3] methods. This approach performs a hit into the filesystem, contrary to its implementation in any other operating system. (({File.expand_path})) is heavily used by (({require})), which result in slow application startup, depending on the application size or number of gems it depends on. Stepping back for a second, we can see that the security issue is around WEBrick and the way it determines (({path_info})) absoluteness. It is also clear that to solve WEBrick security issue, a tax has been applied to the entire Ruby ecosystem, penalizing startup performance. With Hiroshi Shirosaki's help, we worked on a patch that: * Limit filesystem hit only to WEBrick, using Windows' GetLongPathName [4]. * Use a Windows-specific API to normalize paths * Improve encoding support. What started as an experiment named Fenix [5] has shown great results on a variety of systems. This patch has been integrated in TheCodeShop [6] releases of Ruby 1.9.3 and tested by Ruby-Core developers Hiroshi Shirosaki and myself. == Performance To demonstrate the benefits of this patch, I've used measurements [7] project and both (({core_require_empty})) and (({core_require_nested})) workloads, obtaining the following results: ruby 2.0.0dev (2012-08-03 trunk 36611) [i386-mingw32] Rehearsal ------------------------------------------------------ core_require_empty 1.186000 3.385000 4.571000 ( 4.676267) --------------------------------------------- total: 4.571000sec user system total real core_require_empty 1.217000 3.385000 4.602000 ( 4.643266) Rehearsal ------------------------------------------------------- core_require_nested 1.514000 3.760000 5.274000 ( 5.305303) ---------------------------------------------- total: 5.274000sec user system total real core_require_nested 1.466000 3.713000 5.179000 ( 5.233300) And with patch applied: ruby 2.0.0dev (2012-08-03 trunk 36611) [i386-mingw32] Rehearsal ------------------------------------------------------ core_require_empty 0.765000 1.077000 1.842000 ( 1.887603) --------------------------------------------- total: 1.842000sec user system total real core_require_empty 0.717000 1.123000 1.840000 ( 1.887603) Rehearsal ------------------------------------------------------- core_require_nested 0.717000 1.670000 2.387000 ( 2.480405) ---------------------------------------------- total: 2.387000sec user system total real core_require_nested 0.890000 1.528000 2.418000 ( 2.496004) Benchmarks were performed all on the same hardware and OS: * CPU: Core 2 Duo T7500 @ 2.20Ghz * RAM: 4GB * HDD: 1.5GB RAMdisk (ImDisk) * OS: Windows 7 Ultimate x64 All tests associated (both File and WEBrick ones) pass. Additional tests that exercise specific aspects of new function were added. Patch has been tested also against: * Visual Studio build of Ruby * Ubuntu 12.04 * Mac OSX And the patch didn't affect either build or tests of it. === Real life impact: Rails The biggest Ruby project affected by this is Rails applications. An empty Rails application on startup requires more than 700 files from different gems: V:\emptyapp>ruby script\rails runner -e production "p $LOADED_FEATURES.size" 772 When benchmark startup using w32time [8]: V:\emptyapp>timer ruby script\rails runner -e production "0" real 7.285 system 4.539 user 2.683 And patched Ruby: V:\emptyapp>timer ruby script\rails runner -e production "0" real 2.620 system 0.873 user 1.700 (best result taken from several warm ups). Now, a mid-sized application like Enki [9] which loads 1146 files in production mode, result in: V:\enki>timer ruby script\rails runner -e production "p $LOADED_FEATURES.size" 1146 real 22.620 system 11.497 user 11.076 Almost ((*23 seconds*)), compared to patched version: V:\enki>timer ruby script\rails runner -e production "0" real 11.013 system 1.981 user 8.938 This change also improves performance of (({rake})) inside Rails, from: V:\enki>timer rake -T ... real 8.689 system 0.015 user 0.000 To: V:\enki>timer rake -T ... real 3.307 system 0.000 user 0.031 Making normal operations more accessible. Looking forward for your thoughts on these changes. Thank you. [1] http://www.ruby-lang.org/en/news/2008/03/03/webric... [2] https://github.com/ruby/ruby/blob/trunk/lib/webric... [3] https://github.com/ruby/ruby/blob/trunk/lib/webric... [4] http://msdn.microsoft.com/en-us/library/windows/de... [5] https://github.com/luislavena/fenix [6] http://thecodeshop.github.com/ [7] https://github.com/jonforums/measurements [8] https://github.com/thecodeshop/w32time [9] https://github.com/xaviershay/enki =end
Enable email notification
Enable multi-page viewPlease log in before posting. Registration is free and takes only a minute.
Existing account
(Switch to SSL-encrypted connection)
NEW: Do you have a Google/GoogleMail or Yahoo account? No registration required!
Log in with Google account | Log in with Yahoo account
Log in with Google account | Log in with Yahoo account
No account? Register here.