From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS4713 221.184.0.0/13 X-Spam-Status: No, score=-2.1 required=3.0 tests=BAYES_00,DKIM_ADSP_NXDOMAIN, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,RCVD_IN_DNSWL_MED,SPF_PASS, T_DKIM_INVALID,T_RP_MATCHES_RCVD shortcircuit=no autolearn=no autolearn_force=no version=3.4.0 Received: from neon.ruby-lang.org (neon.ruby-lang.org [221.186.184.75]) by dcvr.yhbt.net (Postfix) with ESMTP id 2CECA1FA14 for ; Fri, 26 May 2017 18:32:15 +0000 (UTC) Received: from neon.ruby-lang.org (localhost [IPv6:::1]) by neon.ruby-lang.org (Postfix) with ESMTP id 8ECED1207C3; Sat, 27 May 2017 03:32:11 +0900 (JST) Received: from o1678948x4.outbound-mail.sendgrid.net (o1678948x4.outbound-mail.sendgrid.net [167.89.48.4]) by neon.ruby-lang.org (Postfix) with ESMTPS id 22DAD1207C1 for ; Sat, 27 May 2017 03:32:08 +0900 (JST) DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=sendgrid.me; h=from:to:references:subject:mime-version:content-type:content-transfer-encoding:list-id; s=smtpapi; bh=qdetu4YzmZj8fiBdsrcOYi6E3uc=; b=rILRkALMeaq+BaHDHK vFg5wJvj9+/p49l1HdCypqjV2Bt7hN2BtDOT8O1E9FdhiJe7bZVd9wyRJiCorYW0 DIPQXFL+5Y4f+P4FIXTNB2LDSdWTiFoZ/+y+kSGoUGWxX4wiTRtQkKDULGjBPeYf XG6aiZR2Ge8WVE7oml06p/vHw= Received: by filter1081p1mdw1.sendgrid.net with SMTP id filter1081p1mdw1-23876-592874A5-1 2017-05-26 18:32:05.025647229 +0000 UTC Received: from herokuapp.com (ec2-54-205-240-123.compute-1.amazonaws.com [54.205.240.123]) by ismtpd0003p1iad1.sendgrid.net (SG) with ESMTP id 910cC99CTsS2f649MjS7XA Fri, 26 May 2017 18:32:04.994 +0000 (UTC) Date: Fri, 26 May 2017 18:32:04 +0000 From: naruse@airemix.jp To: ruby-core@ruby-lang.org Message-ID: References: Mime-Version: 1.0 X-Redmine-MailingListIntegration-Message-Ids: 56429 X-Redmine-Project: ruby-trunk X-Redmine-Issue-Id: 13378 X-Redmine-Issue-Author: burke X-Redmine-Sender: naruse X-Mailer: Redmine X-Redmine-Host: bugs.ruby-lang.org X-Redmine-Site: Ruby Issue Tracking System X-Auto-Response-Suppress: All Auto-Submitted: auto-generated X-SG-EID: ync6xU2WACa70kv/Ymy4QrNMhiuLXJG8OTL2vJD1yS5YWDkYzY+FSbS58ZCUpmh0or2E8zD9470fFc mY5gos65o0uq2TLoD+Gv4lpabS03j4XIhWLjgST7OsAp1lk58rgfAxfxO8vYIiq/4+4TJqzAoMQg+u jPJWVex9qtt1vHdKpyzISx+kgvlo7kWIdffg0rpW9Xx6jz9fxCTPKKquBQ== X-ML-Name: ruby-core X-Mail-Count: 81397 Subject: [ruby-core:81397] [Ruby trunk Feature#13378] Eliminate 4 of 8 syscalls when requiring file by absolute path X-BeenThere: ruby-core@ruby-lang.org X-Mailman-Version: 2.1.15 Precedence: list Reply-To: Ruby developers List-Id: Ruby developers List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ruby-core-bounces@ruby-lang.org Sender: "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/