* [ruby-core:80437] [Ruby trunk Feature#13378] Eliminate 4 of 8 syscalls when requiring file by absolute path
[not found] <redmine.issue-13378.20170328194610@ruby-lang.org>
@ 2017-03-28 19:46 ` burke
2017-03-28 20:31 ` [ruby-core:80438] " burke
` (3 subsequent siblings)
4 siblings, 0 replies; 5+ messages in thread
From: burke @ 2017-03-28 19:46 UTC (permalink / raw)
To: ruby-core
Issue #13378 has been reported by burke (Burke Libbey).
----------------------------------------
Feature #13378: Eliminate 4 of 8 syscalls when requiring file by absolute path
https://bugs.ruby-lang.org/issues/13378
* Author: burke (Burke Libbey)
* Status: Open
* Priority: Normal
* Assignee:
* Target version:
----------------------------------------
Don't open file twice when specified by absolute path.
When invoking `require '/a.rb'` (i.e. via an absolute path), ruby generates this sequence of syscalls:
open /a.rb
fstat64 /a.rb
close /a.rb
open /a.rb
fstat64 /a.rb
fstat64 /a.rb
read /a.rb
close /a.rb
It is apparent that the only inherently necessary members of this sequence are:
open /a.rb
fstat64 /a.rb
read /a.rb
close /a.rb
(the fstat64 isn't *obviously* necessary, but it does serve a purpose and probably shouldn't be removed).
The first open/fstat64/close is used to check whether the file is loadable. This is important when scanning the `$LOAD_PATH`, since it is used to determine when a file has been found. However, when we've already unambiguously identified a file before invoking `require`, this serves no inherent purpose, since we can move whatever work is happening as a result of that `fstat64` into the second open/close sequence.
This change bypasses the first open/fstat64/close in the case of an absolute path to `require`. It also removes one of the doubled-up `fstat64` calls later in the sequence. As a result, the number of syscalls to require a file changes:
* From 8 to 4 when specified by absolute path;
* From 5+3n to 4+3n otherwise *(where n is the number of `$LOAD_PATH` items scanned)*.
In future work, it would be possible to re-use the file descriptor opened while searching the `$LOAD_PATH` without the close/open sequence, but this would cause some ugly layering issues.
---
*We intend to use this in conjunction with something like https://github.com/shopify/bootscale, which pre-resolves required features to absolute paths before calling `require`. This change reduces our total number of filesystem accesses by 13% during application boot.*
*Various notes and rationale at http://notes.burke.libbey.me/ruby-require-optimization*
---Files--------------------------------
0001-reduce-syscalls-on-require.patch (7.56 KB)
--
https://bugs.ruby-lang.org/
^ permalink raw reply [flat|nested] 5+ messages in thread
* [ruby-core:80438] [Ruby trunk Feature#13378] Eliminate 4 of 8 syscalls when requiring file by absolute path
[not found] <redmine.issue-13378.20170328194610@ruby-lang.org>
2017-03-28 19:46 ` [ruby-core:80437] [Ruby trunk Feature#13378] Eliminate 4 of 8 syscalls when requiring file by absolute path burke
@ 2017-03-28 20:31 ` burke
2017-05-26 18:32 ` [ruby-core:81397] " naruse
` (2 subsequent siblings)
4 siblings, 0 replies; 5+ messages in thread
From: burke @ 2017-03-28 20:31 UTC (permalink / raw)
To: ruby-core
Issue #13378 has been updated by burke (Burke Libbey).
File 0001-reduce-syscalls-on-require-fixed.patch added
Ah, I was too hasty with the rebase from my 2.3.3 branch. I've attached a fixed patch. Note also that trunk has already eliminated the double-fstat, so this only reduces the number of syscalls when a feature is specified by absolute path (from 7 to 4).
----------------------------------------
Feature #13378: Eliminate 4 of 8 syscalls when requiring file by absolute path
https://bugs.ruby-lang.org/issues/13378#change-63929
* Author: burke (Burke Libbey)
* Status: Open
* Priority: Normal
* Assignee:
* Target version:
----------------------------------------
Don't open file twice when specified by absolute path.
When invoking `require '/a.rb'` (i.e. via an absolute path), ruby generates this sequence of syscalls:
open /a.rb
fstat64 /a.rb
close /a.rb
open /a.rb
fstat64 /a.rb
fstat64 /a.rb
read /a.rb
close /a.rb
It is apparent that the only inherently necessary members of this sequence are:
open /a.rb
fstat64 /a.rb
read /a.rb
close /a.rb
(the fstat64 isn't *obviously* necessary, but it does serve a purpose and probably shouldn't be removed).
The first open/fstat64/close is used to check whether the file is loadable. This is important when scanning the `$LOAD_PATH`, since it is used to determine when a file has been found. However, when we've already unambiguously identified a file before invoking `require`, this serves no inherent purpose, since we can move whatever work is happening as a result of that `fstat64` into the second open/close sequence.
This change bypasses the first open/fstat64/close in the case of an absolute path to `require`. It also removes one of the doubled-up `fstat64` calls later in the sequence. As a result, the number of syscalls to require a file changes:
* From 8 to 4 when specified by absolute path;
* From 5+3n to 4+3n otherwise *(where n is the number of `$LOAD_PATH` items scanned)*.
In future work, it would be possible to re-use the file descriptor opened while searching the `$LOAD_PATH` without the close/open sequence, but this would cause some ugly layering issues.
---
*We intend to use this in conjunction with something like https://github.com/shopify/bootscale, which pre-resolves required features to absolute paths before calling `require`. This change reduces our total number of filesystem accesses by 13% during application boot.*
*Various notes and rationale at http://notes.burke.libbey.me/ruby-require-optimization*
---Files--------------------------------
0001-reduce-syscalls-on-require.patch (7.56 KB)
0001-reduce-syscalls-on-require-fixed.patch (6.94 KB)
--
https://bugs.ruby-lang.org/
^ permalink raw reply [flat|nested] 5+ messages in thread
* [ruby-core:81397] [Ruby trunk Feature#13378] Eliminate 4 of 8 syscalls when requiring file by absolute path
[not found] <redmine.issue-13378.20170328194610@ruby-lang.org>
2017-03-28 19:46 ` [ruby-core:80437] [Ruby trunk Feature#13378] Eliminate 4 of 8 syscalls when requiring file by absolute path burke
2017-03-28 20:31 ` [ruby-core:80438] " burke
@ 2017-05-26 18:32 ` naruse
2017-05-29 18:56 ` [ruby-core:81460] " burke
2017-06-16 7:57 ` [ruby-core:81702] " ko1
4 siblings, 0 replies; 5+ messages in thread
From: naruse @ 2017-05-26 18:32 UTC (permalink / raw)
To: ruby-core
Issue #13378 has been updated by naruse (Yui NARUSE).
> -VALUE rb_find_file_safe(VALUE, int);
> +VALUE rb_find_file_safe(VALUE, int, int);
When you are CRuby developer, all functions declared under include/**/*.h are considered public C API.
(Note that if you are C extension developer, some of such APIs are experimental or private...)
Therefore rb_find_file_safe() should be kept as is.
You should declare rb_find_file_safe_with_defer_load_check or something in internal.h (not include/ruby/intern.h).
Or no one seems to use the function by GitHub search https://github.com/search?utf8=%E2%9C%93&q=rb_find_file_safe++extension%3Ac+path%3Aext&type=Code
we can simply change the prototype and move to internal.h.
> if ((fd = rb_cloexec_open(fname, mode, 0)) < 0) {
> - rb_load_fail(fname_v, strerror(errno));
> + goto fail;
> }
This seems to need `e = errno;` before goto fail;
> int fd = 0;
It should be -1 because 0 is still valid fd even though it is STDIN.
> if (fd > 0) (void)close(fd);
ditto.
Additionally, if you have tests to confirm the behavior, could you add them to test/ruby/test_require.rb?
----------------------------------------
Feature #13378: Eliminate 4 of 8 syscalls when requiring file by absolute path
https://bugs.ruby-lang.org/issues/13378#change-65109
* Author: burke (Burke Libbey)
* Status: Open
* Priority: Normal
* Assignee:
* Target version:
----------------------------------------
Don't open file twice when specified by absolute path.
When invoking `require '/a.rb'` (i.e. via an absolute path), ruby generates this sequence of syscalls:
open /a.rb
fstat64 /a.rb
close /a.rb
open /a.rb
fstat64 /a.rb
fstat64 /a.rb
read /a.rb
close /a.rb
It is apparent that the only inherently necessary members of this sequence are:
open /a.rb
fstat64 /a.rb
read /a.rb
close /a.rb
(the fstat64 isn't *obviously* necessary, but it does serve a purpose and probably shouldn't be removed).
The first open/fstat64/close is used to check whether the file is loadable. This is important when scanning the `$LOAD_PATH`, since it is used to determine when a file has been found. However, when we've already unambiguously identified a file before invoking `require`, this serves no inherent purpose, since we can move whatever work is happening as a result of that `fstat64` into the second open/close sequence.
This change bypasses the first open/fstat64/close in the case of an absolute path to `require`. It also removes one of the doubled-up `fstat64` calls later in the sequence. As a result, the number of syscalls to require a file changes:
* From 8 to 4 when specified by absolute path;
* From 5+3n to 4+3n otherwise *(where n is the number of `$LOAD_PATH` items scanned)*.
In future work, it would be possible to re-use the file descriptor opened while searching the `$LOAD_PATH` without the close/open sequence, but this would cause some ugly layering issues.
---
*We intend to use this in conjunction with something like https://github.com/shopify/bootscale, which pre-resolves required features to absolute paths before calling `require`. This change reduces our total number of filesystem accesses by 13% during application boot.*
*Various notes and rationale at http://notes.burke.libbey.me/ruby-require-optimization*
---Files--------------------------------
0001-reduce-syscalls-on-require.patch (7.56 KB)
0001-reduce-syscalls-on-require-fixed.patch (6.94 KB)
--
https://bugs.ruby-lang.org/
^ permalink raw reply [flat|nested] 5+ messages in thread
* [ruby-core:81460] [Ruby trunk Feature#13378] Eliminate 4 of 8 syscalls when requiring file by absolute path
[not found] <redmine.issue-13378.20170328194610@ruby-lang.org>
` (2 preceding siblings ...)
2017-05-26 18:32 ` [ruby-core:81397] " naruse
@ 2017-05-29 18:56 ` burke
2017-06-16 7:57 ` [ruby-core:81702] " ko1
4 siblings, 0 replies; 5+ messages in thread
From: burke @ 2017-05-29 18:56 UTC (permalink / raw)
To: ruby-core
Issue #13378 has been updated by burke (Burke Libbey).
File 0001-reduce-syscalls-on-require-v2.patch added
Thank you for the feedback! I've attached an updated patch to address the issues.
As for testing it, I haven't been able to think of a reasonable method to verify the behaviour without using dtrace/strace, since the only observable effect without system-level instrumentation should be a slight reduction in runtime. I *could* add a case to `DTrace::TestRequire`, but it feels wrong, since this file is concerned with testing the dtrace probes implemented by ruby, not testing ruby using built-in dtrace probes.
Suggestions?
----------------------------------------
Feature #13378: Eliminate 4 of 8 syscalls when requiring file by absolute path
https://bugs.ruby-lang.org/issues/13378#change-65163
* Author: burke (Burke Libbey)
* Status: Open
* Priority: Normal
* Assignee:
* Target version:
----------------------------------------
Don't open file twice when specified by absolute path.
When invoking `require '/a.rb'` (i.e. via an absolute path), ruby generates this sequence of syscalls:
open /a.rb
fstat64 /a.rb
close /a.rb
open /a.rb
fstat64 /a.rb
fstat64 /a.rb
read /a.rb
close /a.rb
It is apparent that the only inherently necessary members of this sequence are:
open /a.rb
fstat64 /a.rb
read /a.rb
close /a.rb
(the fstat64 isn't *obviously* necessary, but it does serve a purpose and probably shouldn't be removed).
The first open/fstat64/close is used to check whether the file is loadable. This is important when scanning the `$LOAD_PATH`, since it is used to determine when a file has been found. However, when we've already unambiguously identified a file before invoking `require`, this serves no inherent purpose, since we can move whatever work is happening as a result of that `fstat64` into the second open/close sequence.
This change bypasses the first open/fstat64/close in the case of an absolute path to `require`. It also removes one of the doubled-up `fstat64` calls later in the sequence. As a result, the number of syscalls to require a file changes:
* From 8 to 4 when specified by absolute path;
* From 5+3n to 4+3n otherwise *(where n is the number of `$LOAD_PATH` items scanned)*.
In future work, it would be possible to re-use the file descriptor opened while searching the `$LOAD_PATH` without the close/open sequence, but this would cause some ugly layering issues.
---
*We intend to use this in conjunction with something like https://github.com/shopify/bootscale, which pre-resolves required features to absolute paths before calling `require`. This change reduces our total number of filesystem accesses by 13% during application boot.*
*Various notes and rationale at http://notes.burke.libbey.me/ruby-require-optimization*
---Files--------------------------------
0001-reduce-syscalls-on-require.patch (7.56 KB)
0001-reduce-syscalls-on-require-fixed.patch (6.94 KB)
0001-reduce-syscalls-on-require-v2.patch (6.44 KB)
--
https://bugs.ruby-lang.org/
^ permalink raw reply [flat|nested] 5+ messages in thread
* [ruby-core:81702] [Ruby trunk Feature#13378] Eliminate 4 of 8 syscalls when requiring file by absolute path
[not found] <redmine.issue-13378.20170328194610@ruby-lang.org>
` (3 preceding siblings ...)
2017-05-29 18:56 ` [ruby-core:81460] " burke
@ 2017-06-16 7:57 ` ko1
4 siblings, 0 replies; 5+ messages in thread
From: ko1 @ 2017-06-16 7:57 UTC (permalink / raw)
To: ruby-core
Issue #13378 has been updated by ko1 (Koichi Sasada).
Assignee set to nobu (Nobuyoshi Nakada)
----------------------------------------
Feature #13378: Eliminate 4 of 8 syscalls when requiring file by absolute path
https://bugs.ruby-lang.org/issues/13378#change-65392
* Author: burke (Burke Libbey)
* Status: Open
* Priority: Normal
* Assignee: nobu (Nobuyoshi Nakada)
* Target version:
----------------------------------------
Don't open file twice when specified by absolute path.
When invoking `require '/a.rb'` (i.e. via an absolute path), ruby generates this sequence of syscalls:
open /a.rb
fstat64 /a.rb
close /a.rb
open /a.rb
fstat64 /a.rb
fstat64 /a.rb
read /a.rb
close /a.rb
It is apparent that the only inherently necessary members of this sequence are:
open /a.rb
fstat64 /a.rb
read /a.rb
close /a.rb
(the fstat64 isn't *obviously* necessary, but it does serve a purpose and probably shouldn't be removed).
The first open/fstat64/close is used to check whether the file is loadable. This is important when scanning the `$LOAD_PATH`, since it is used to determine when a file has been found. However, when we've already unambiguously identified a file before invoking `require`, this serves no inherent purpose, since we can move whatever work is happening as a result of that `fstat64` into the second open/close sequence.
This change bypasses the first open/fstat64/close in the case of an absolute path to `require`. It also removes one of the doubled-up `fstat64` calls later in the sequence. As a result, the number of syscalls to require a file changes:
* From 8 to 4 when specified by absolute path;
* From 5+3n to 4+3n otherwise *(where n is the number of `$LOAD_PATH` items scanned)*.
In future work, it would be possible to re-use the file descriptor opened while searching the `$LOAD_PATH` without the close/open sequence, but this would cause some ugly layering issues.
---
*We intend to use this in conjunction with something like https://github.com/shopify/bootscale, which pre-resolves required features to absolute paths before calling `require`. This change reduces our total number of filesystem accesses by 13% during application boot.*
*Various notes and rationale at http://notes.burke.libbey.me/ruby-require-optimization*
---Files--------------------------------
0001-reduce-syscalls-on-require.patch (7.56 KB)
0001-reduce-syscalls-on-require-fixed.patch (6.94 KB)
0001-reduce-syscalls-on-require-v2.patch (6.44 KB)
--
https://bugs.ruby-lang.org/
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-06-16 7:57 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <redmine.issue-13378.20170328194610@ruby-lang.org>
2017-03-28 19:46 ` [ruby-core:80437] [Ruby trunk Feature#13378] Eliminate 4 of 8 syscalls when requiring file by absolute path burke
2017-03-28 20:31 ` [ruby-core:80438] " burke
2017-05-26 18:32 ` [ruby-core:81397] " naruse
2017-05-29 18:56 ` [ruby-core:81460] " burke
2017-06-16 7:57 ` [ruby-core:81702] " ko1
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).