Project

General

Profile

Actions

Feature #19465

open

[PATCH] reuse open(2) from rb_file_load_ok on POSIX-like system

Added by normalperson (Eric Wong) about 1 year ago. Updated about 1 year ago.

Status:
Assigned
Target version:
-
[ruby-core:112584]

Description

When loading Ruby source files, we can save the result of
successful opens as open(2)/openat(2) are a fairly expensive
syscalls.  This also avoids a time-of-check-to-time-of-use
(TOCTTOU) problem.

This reduces open(2) syscalls during `require'; but should be
most apparent when users have a small $LOAD_PATH.  Users with
large $LOAD_PATH will benefit less since there'll be more
open(2) failures due to ENOENT.

With `strace -c -e openat ruby -e exit' under Linux, this
results in a ~14% reduction of openat(2) syscalls
(glibc uses openat(2) to implement open(2)).

 % time     seconds  usecs/call     calls    errors syscall
 ------ ----------- ----------- --------- --------- ----------------
   0.00    0.000000           0       296       110 openat
   0.00    0.000000           0       254       110 openat

Additionally, the introduction of `struct ruby_file_load_state'
may make future optimizations more apparent.

This change cannot benefit binary (.so) loading since the
dlopen(3) API requires a filename and I'm not aware of an
alternative that takes a pre-existing FD.  In typical
situations, Ruby source files outnumber the mount of .so
files.


I've only tested this lightly on small apps since I don't have
large codebases to test on.  However, I think organizing various
on-stack variables into `struct ruby_file_load_state' can be
beneficial if we end up using io-uring on Linux.

Files

Updated by nobu (Nobuyoshi Nakada) about 1 year ago

Seems fine.👍

Actions #2

Updated by Anonymous about 1 year ago

"nobu (Nobuyoshi Nakada) via ruby-core" wrote:

Issue #19465 has been updated by nobu (Nobuyoshi Nakada).
https://bugs.ruby-lang.org/issues/19465#change-102042

Seems fine.

OK, thanks, committed as 35136e1e9c232ad7a03407b992b2e86b6df43f63
Hopefully I didn't break anything for win32.

Btw, to display your emoji as ASCII in my text terminal with
limited glyphs, I had use the following Perl script.
Not sure if there's a way to do this with Ruby stdlib:
--------8<------
eval 'exec perl -w -S $0 ${1+"$@"}'
if 0; # running under some shell
use v5.12;
use charnames ();
use Encode qw(find_encoding);
binmode STDIN, ':utf8';
binmode STDOUT, ':utf8';
my $enc_ascii = find_encoding('us-ascii');
my $fallback = sub {
	join('', map {
		if ($_ == 0xfe0f) { # VARIATION SELECTOR-16
			'';
		} elsif (defined(my $name = charnames::viacode($_))) {
			"<$name>"
		} else {
			sprintf('<#0x%x>', $_);
		}
	} @_);
};
while (<STDIN>) { print $enc_ascii->encode($_, $fallback) }

Actions #3

Updated by byroot (Jean Boussier) about 1 year ago

  • Status changed from Open to Closed

Updated by k0kubun (Takashi Kokubun) about 1 year ago

  • Status changed from Closed to Assigned
  • Assignee set to normalperson (Eric Wong)

Since there has been no action since @hsbt (Hiroshi SHIBATA) reported it, I temporarily reverted the commit at 526111290b2e01e798f436dfe4acc3bf10c6bbd1. Please have a look at the failure and re-push it once the issue is resolved. Thank you!

Updated by hsbt (Hiroshi SHIBATA) about 1 year ago

Copy Eric's message from ruby-core:112626

35136e1e9c232ad7a03407b992b2e86b6df43f63 is broken with gcc-11 annocheck.

Its configuration is here:
https://git.ruby-lang.org/ruby.git/tree/.github/workflows/compilers.yml#n82

  default_cc: 'gcc-11 -fcf-protection -Wa,--generate-missing-build-notes=yes'
  optflags: '-O2'
  LDFLAGS: '-Wl,-z,now'

OK, only gcc11-available system I have is a FreeBSD VM and I
couldn't reproduce it with those flags, (no podman/docker on
FreeBSD).

69 1) An exception occurred during: Kernel#require_relative with a
relative path (file extensions) does not load a C-extension file if a
complex-extensioned .rb file is already loaded
70 /__w/ruby/ruby/src/spec/ruby/core/kernel/require_relative_spec.rb:197
71 Kernel#require_relative with a relative path (file extensions) does
not load a C-extension file if a complex-extensioned .rb file is
already loaded ERROR
72 LeakError: Leaked file descriptor: 8 :
#File:/__w/ruby/ruby/src/spec/ruby/fixtures/code/load_fixture.ext.rb

I saw similar errors in development, but thought I fixed them all.
Where there other CI instances which failed or just the annocheck one?
I can't find failures for 35136e1e9 in http://ci.rvm.jp/failure_results

Is there a machine/VM I can ssh/mosh into which reproduces that issue?

Thanks.

Actions

Also available in: Atom PDF

Like1
Like0Like0Like0Like0Like0