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: AS37560 197.231.220.0/22 X-Spam-Status: No, score=-2.6 required=3.0 tests=BAYES_00,RCVD_IN_MSPIKE_BL, RCVD_IN_MSPIKE_ZBI,RCVD_IN_XBL,SPF_FAIL,SPF_HELO_FAIL,TO_EQ_FM_DOM_SPF_FAIL shortcircuit=no autolearn=no autolearn_force=no version=3.4.0 Received: from 80x24.org (exit1.ipredator.se [197.231.221.211]) by dcvr.yhbt.net (Postfix) with ESMTP id 6E9DE2047F for ; Fri, 29 Sep 2017 08:50:27 +0000 (UTC) From: Eric Wong To: spew@80x24.org Subject: [PATCH] File#rename releases GVL Date: Fri, 29 Sep 2017 08:50:16 +0000 Message-Id: <20170929085016.9441-1-e@80x24.org> List-Id: rename(2) requires two pathname resolution operations which can take considerable time on slow filesystems, release the GVL so operations on other threads may proceed. On fast, local filesystems, this change results in some slowdown as shown by the new benchmark. I consider the performance trade off acceptable as cases are avoided. benchmark results: minimum results in each 3 measurements. Execution time (sec) name trunk built file_rename 2.648 2.804 Speedup ratio: compare with the result of `trunk' (greater is better) name built file_rename 0.944 * file.c (no_gvl_rename): new function (rb_file_s_rename): release GVL for renames * benchmark/bm_file_rename.rb: new benchmark --- benchmark/bm_file_rename.rb | 11 +++++++++++ file.c | 22 ++++++++++++++++++---- 2 files changed, 29 insertions(+), 4 deletions(-) create mode 100644 benchmark/bm_file_rename.rb diff --git a/benchmark/bm_file_rename.rb b/benchmark/bm_file_rename.rb new file mode 100644 index 0000000000..3bf6a5ef35 --- /dev/null +++ b/benchmark/bm_file_rename.rb @@ -0,0 +1,11 @@ +# rename file +require 'tempfile' + +max = 100_000 +tmp = [ Tempfile.new('rename-a'), Tempfile.new('rename-b') ] +a, b = tmp.map { |x| x.path } +max.times do + File.rename(a, b) + File.rename(b, a) +end +tmp.each { |t| t.close! } diff --git a/file.c b/file.c index deb666e096..042ffdbe6a 100644 --- a/file.c +++ b/file.c @@ -2873,6 +2873,19 @@ rb_file_s_unlink(int argc, VALUE *argv, VALUE klass) return apply2files(unlink_internal, argc, argv, 0); } +struct rename_args { + const char *src; + const char *dst; +}; + +static void * +no_gvl_rename(void *ptr) +{ + struct rename_args *ra = ptr; + + return (void *)rename(ra->src, ra->dst); +} + /* * call-seq: * File.rename(old_name, new_name) -> 0 @@ -2886,19 +2899,20 @@ rb_file_s_unlink(int argc, VALUE *argv, VALUE klass) static VALUE rb_file_s_rename(VALUE klass, VALUE from, VALUE to) { - const char *src, *dst; + struct rename_args ra; VALUE f, t; FilePathValue(from); FilePathValue(to); f = rb_str_encode_ospath(from); t = rb_str_encode_ospath(to); - src = StringValueCStr(f); - dst = StringValueCStr(t); + ra.src = StringValueCStr(f); + ra.dst = StringValueCStr(t); #if defined __CYGWIN__ errno = 0; #endif - if (rename(src, dst) < 0) { + if ((int)rb_thread_call_without_gvl(no_gvl_rename, &ra, + RUBY_UBF_IO, 0) < 0) { int e = errno; #if defined DOSISH switch (e) { -- EW