ruby-changes:48271
From: normal <ko1@a...>
Date: Tue, 24 Oct 2017 10:20:11 +0900 (JST)
Subject: [ruby-changes:48271] normal:r60386 (trunk): file.c: apply2files releases GVL
normal 2017-10-24 10:20:04 +0900 (Tue, 24 Oct 2017) New Revision: 60386 https://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=revision&revision=60386 Log: file.c: apply2files releases GVL This means File.chmod, File.lchmod, File.chown, File.lchown, File.unlink, and File.utime operations on slow filesystems no longer hold up other threads. The platform-specific utime_failed changes is compile-tested using a new UTIME_EINVAL macro This hurts performance on fast filesystem, but these methods are unlikely to be performance bottlenecks and (IMHO) avoiding pathological slowdowns and stalls are more important. benchmark results: minimum results in each 3 measurements. Execution time (sec) name trunk built file_chmod 0.591 0.801 Speedup ratio: compare with the result of `trunk' (greater is better) name built file_chmod 0.737 * file.c (UTIME_EINVAL): new macro to ease compile-testing * file.c (struct apply_arg): new struct * file.c (no_gvl_apply2files): new function * file.c (apply2files): release GVL * file.c (chmod_internal): adjust for apply2files changes * file.c (lchmod_internal): ditto * file.c (chown_internal): ditto * file.c (lchown_internal): ditto * file.c (utime_failed): ditto * file.c (utime_internal): ditto * file.c (unlink_internal): ditto [ruby-core:83200] [Feature #13996] Added files: trunk/benchmark/bm_file_chmod.rb Modified files: trunk/file.c Index: benchmark/bm_file_chmod.rb =================================================================== --- benchmark/bm_file_chmod.rb (nonexistent) +++ benchmark/bm_file_chmod.rb (revision 60386) @@ -0,0 +1,9 @@ https://github.com/ruby/ruby/blob/trunk/benchmark/bm_file_chmod.rb#L1 +# chmod file +require 'tempfile' +max = 200_000 +tmp = Tempfile.new('chmod') +path = tmp.path +max.times do + File.chmod(0777, path) +end +tmp.close! Index: file.c =================================================================== --- file.c (revision 60385) +++ file.c (revision 60386) @@ -126,6 +126,11 @@ int flock(int, int); https://github.com/ruby/ruby/blob/trunk/file.c#L126 # define TO_OSPATH(str) (str) #endif +/* utime may fail if time is out-of-range for the FS [ruby-dev:38277] */ +#if defined DOSISH || defined __CYGWIN__ +# define UTIME_EINVAL +#endif + VALUE rb_cFile; VALUE rb_mFileTest; VALUE rb_cStat; @@ -344,20 +349,87 @@ ignored_char_p(const char *p, const char https://github.com/ruby/ruby/blob/trunk/file.c#L349 #define apply2args(n) (rb_check_arity(argc, n, UNLIMITED_ARGUMENTS), argc-=n) +struct apply_arg { + int i; + int argc; + int errnum; + int (*func)(const char *, void *); + void *arg; + struct { + const char *ptr; + VALUE path; + } fn[1]; /* flexible array */ +}; + +static void * +no_gvl_apply2files(void *ptr) +{ + struct apply_arg *aa = ptr; + + for (aa->i = 0; aa->i < aa->argc; aa->i++) { + if (aa->func(aa->fn[aa->i].ptr, aa->arg) < 0) { + aa->errnum = errno; + break; + } + } + return 0; +} + +#ifdef UTIME_EINVAL +NORETURN(static void utime_failed(struct apply_arg *)); +static int utime_internal(const char *, void *); +#endif + static VALUE -apply2files(void (*func)(const char *, VALUE, void *), int argc, VALUE *argv, void *arg) +apply2files(int (*func)(const char *, void *), int argc, VALUE *argv, void *arg) { - long i; - VALUE path; + VALUE v; + VALUE tmp = Qfalse; + size_t size = sizeof(const char *) + sizeof(VALUE); + const long len = (long)(sizeof(struct apply_arg) + (size * argc) - size); + struct apply_arg *aa = ALLOCV(v, len); + + aa->errnum = 0; + aa->argc = argc; + aa->arg = arg; + aa->func = func; + + /* + * aa is on-stack for small argc, we must ensure paths are marked + * for large argv if aa is on the heap. + */ + if (v) { + tmp = rb_ary_tmp_new(argc); + } + + for (aa->i = 0; aa->i < argc; aa->i++) { + VALUE path = rb_get_path(argv[aa->i]); - for (i=0; i<argc; i++) { - const char *s; - path = rb_get_path(argv[i]); path = rb_str_encode_ospath(path); - s = RSTRING_PTR(path); - (*func)(s, path, arg); + aa->fn[aa->i].ptr = RSTRING_PTR(path); + aa->fn[aa->i].path = path; + + if (tmp != Qfalse) { + rb_ary_push(tmp, path); + } } + rb_thread_call_without_gvl(no_gvl_apply2files, aa, RUBY_UBF_IO, 0); + if (aa->errnum) { +#ifdef UTIME_EINVAL + if (func == utime_internal) { + utime_failed(aa); + } +#endif + rb_syserr_fail_path(aa->errnum, aa->fn[aa->i].path); + } + if (v) { + ALLOCV_END(v); + } + if (tmp != Qfalse) { + rb_ary_clear(tmp); + rb_gc_force_recycle(tmp); + } return LONG2FIX(argc); } @@ -2328,11 +2400,10 @@ rb_file_size(VALUE obj) https://github.com/ruby/ruby/blob/trunk/file.c#L2400 return OFFT2NUM(st.st_size); } -static void -chmod_internal(const char *path, VALUE pathv, void *mode) +static int +chmod_internal(const char *path, void *mode) { - if (chmod(path, *(int *)mode) < 0) - rb_sys_fail_path(pathv); + return chmod(path, *(int *)mode); } /* @@ -2404,11 +2475,10 @@ rb_file_chmod(VALUE obj, VALUE vmode) https://github.com/ruby/ruby/blob/trunk/file.c#L2475 } #if defined(HAVE_LCHMOD) -static void -lchmod_internal(const char *path, VALUE pathv, void *mode) +static int +lchmod_internal(const char *path, void *mode) { - if (lchmod(path, (int)(VALUE)mode) < 0) - rb_sys_fail_path(pathv); + return lchmod(path, (int)(VALUE)mode); } /* @@ -2458,12 +2528,11 @@ struct chown_args { https://github.com/ruby/ruby/blob/trunk/file.c#L2528 rb_gid_t group; }; -static void -chown_internal(const char *path, VALUE pathv, void *arg) +static int +chown_internal(const char *path, void *arg) { struct chown_args *args = arg; - if (chown(path, args->owner, args->group) < 0) - rb_sys_fail_path(pathv); + return chown(path, args->owner, args->group); } /* @@ -2535,12 +2604,11 @@ rb_file_chown(VALUE obj, VALUE owner, VA https://github.com/ruby/ruby/blob/trunk/file.c#L2604 } #if defined(HAVE_LCHOWN) -static void -lchown_internal(const char *path, VALUE pathv, void *arg) +static int +lchown_internal(const char *path, void *arg) { struct chown_args *args = arg; - if (lchown(path, args->owner, args->group) < 0) - rb_sys_fail_path(pathv); + return lchown(path, args->owner, args->group); } /* @@ -2574,16 +2642,22 @@ struct utime_args { https://github.com/ruby/ruby/blob/trunk/file.c#L2642 VALUE atime, mtime; }; -#if defined DOSISH || defined __CYGWIN__ -NORETURN(static void utime_failed(VALUE, const struct timespec *, VALUE, VALUE)); +#ifdef UTIME_EINVAL +NORETURN(static void utime_failed(struct apply_arg *)); static void -utime_failed(VALUE path, const struct timespec *tsp, VALUE atime, VALUE mtime) +utime_failed(struct apply_arg *aa) { - int e = errno; - if (tsp && e == EINVAL) { + int e = aa->errnum; + VALUE path = aa->fn[aa->i].path; + struct utime_args *ua = aa->arg; + + if (ua->tsp && e == EINVAL) { VALUE e[2], a = Qnil, m = Qnil; int d = 0; + VALUE atime = ua->atime; + VALUE mtime = ua->mtime; + if (!NIL_P(atime)) { a = rb_inspect(atime); } @@ -2608,14 +2682,12 @@ utime_failed(VALUE path, const struct ti https://github.com/ruby/ruby/blob/trunk/file.c#L2682 } rb_syserr_fail_path(e, path); } -#else -#define utime_failed(path, tsp, atime, mtime) rb_sys_fail_path(path) #endif #if defined(HAVE_UTIMES) -static void -utime_internal(const char *path, VALUE pathv, void *arg) +static int +utime_internal(const char *path, void *arg) { struct utime_args *v = arg; const struct timespec *tsp = v->tsp; @@ -2630,9 +2702,9 @@ utime_internal(const char *path, VALUE p https://github.com/ruby/ruby/blob/trunk/file.c#L2702 try_utimensat = 0; goto no_utimensat; } - utime_failed(pathv, tsp, v->atime, v->mtime); + return -1; /* calls utime_failed */ } - return; + return 0; } no_utimensat: #endif @@ -2644,8 +2716,7 @@ no_utimensat: https://github.com/ruby/ruby/blob/trunk/file.c#L2716 tvbuf[1].tv_usec = (int)(tsp[1].tv_nsec / 1000); tvp = tvbuf; } - if (utimes(path, tvp) < 0) - utime_failed(pathv, tsp, v->atime, v->mtime); + return utimes(path, tvp); } #else @@ -2657,8 +2728,8 @@ struct utimbuf { https://github.com/ruby/ruby/blob/trunk/file.c#L2728 }; #endif -static void -utime_internal(const char *path, VALUE pathv, void *arg) +static int +utime_internal(const char *path, void *arg) { struct utime_args *v = arg; const struct timespec *tsp = v->tsp; @@ -2668,8 +2739,7 @@ utime_internal(const char *path, VALUE p https://github.com/ruby/ruby/blob/trunk/file.c#L2739 utbuf.modtime = tsp[1].tv_sec; utp = &utbuf; } - if (utime(path, utp) < 0) - utime_failed(pathv, tsp, v->atime, v->mtime); + return utime(path, utp); } #endif @@ -2850,11 +2920,10 @@ rb_readlink(VALUE path, rb_encoding *enc https://github.com/ruby/ruby/blob/trunk/file.c#L2920 #define rb_file_s_readlink rb_f_notimplement #endif -static void -unlink_internal(const char *path, VALUE pathv, void *arg) +static int +unlink_internal(const char *path, void *arg) { - if (unlink(path) < 0) - rb_sys_fail_path(pathv); + return unlink(path); } /* -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/