From eec16d3c6b8a750c5fa46e16b829c2a8b04c6974 Mon Sep 17 00:00:00 2001 From: Nobuyoshi Nakada Date: Wed, 4 Oct 2023 22:55:24 +0900 Subject: [PATCH 1/5] Remove magic numbers from `show_usage_line` --- ruby.c | 41 +++++++++++++++++++++++------------------ 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/ruby.c b/ruby.c index 44b1e06031f678..2242f45cfa7818 100644 --- a/ruby.c +++ b/ruby.c @@ -241,41 +241,46 @@ static const char esc_standout[] = "\n\033[1;7m"; static const char esc_bold[] = "\033[1m"; static const char esc_reset[] = "\033[0m"; static const char esc_none[] = ""; +#define USAGE_INDENT " " /* macro for concatenation */ static void show_usage_line(const struct ruby_opt_message *m, int help, int highlight, unsigned int w, int columns) { + static const int indent_width = (int)rb_strlen_lit(USAGE_INDENT); const char *str = m->str; - const unsigned int namelen = m->namelen, secondlen = m->secondlen; + const char *str2 = str + m->namelen; + const char *desc = str + m->namelen + m->secondlen; + const unsigned int namelen = m->namelen - 1, secondlen = m->secondlen - 1; const char *sb = highlight ? esc_bold : esc_none; const char *se = highlight ? esc_reset : esc_none; - const char *desc = str + namelen + secondlen; unsigned int desclen = (unsigned int)strcspn(desc, "\n"); - if (help && (namelen > w) && (int)(namelen + secondlen) >= columns) { - printf(" %s" "%.*s" "%s\n", sb, namelen-1, str, se); - if (secondlen > 1) { - const int second_end = namelen+secondlen-1; - int n = namelen; - if (str[n] == ',') n++; - if (str[n] == ' ') n++; - printf(" %s" "%.*s" "%s\n", sb, second_end-n, str+n, se); + if (help && (namelen + 1 > w) && /* a padding space */ + (int)(namelen + secondlen + indent_width) >= columns) { + printf(USAGE_INDENT "%s" "%.*s" "%s\n", sb, namelen, str, se); + if (secondlen > 0) { + const int second_end = secondlen; + int n = 0; + if (str2[n] == ',') n++; + if (str2[n] == ' ') n++; + printf(USAGE_INDENT "%s" "%.*s" "%s\n", sb, second_end-n, str2+n, se); } - printf("%-*s%.*s\n", w + 2, "", desclen, desc); + printf("%-*s%.*s\n", w + indent_width, USAGE_INDENT, desclen, desc); } else { - const int wrap = help && namelen + secondlen - 1 > w; - printf(" %s%.*s%-*.*s%s%-*s%.*s\n", sb, namelen-1, str, - (wrap ? 0 : w - namelen + 1), - (help ? secondlen-1 : 0), str + namelen, se, - (wrap ? w + 3 : 0), (wrap ? "\n" : ""), + const int wrap = help && namelen + secondlen >= w; + printf(USAGE_INDENT "%s%.*s%-*.*s%s%-*s%.*s\n", sb, namelen, str, + (wrap ? 0 : w - namelen), + (help ? secondlen : 0), str2, se, + (wrap ? (int)(w + rb_strlen_lit("\n" USAGE_INDENT)) : 0), + (wrap ? "\n" USAGE_INDENT : ""), desclen, desc); } if (help) { while (desc[desclen]) { - desc += desclen + 1; + desc += desclen + rb_strlen_lit("\n"); desclen = (unsigned int)strcspn(desc, "\n"); - printf("%-*s%.*s\n", w + 2, "", desclen, desc); + printf("%-*s%.*s\n", w + indent_width, USAGE_INDENT, desclen, desc); } } } From 22bd9d2a5ad22c964cea21119fc9551e710b5433 Mon Sep 17 00:00:00 2001 From: Nobuyoshi Nakada Date: Wed, 4 Oct 2023 22:57:56 +0900 Subject: [PATCH 2/5] Split `show_usage_line` and add `ruby_show_usage_line` --- ruby.c | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/ruby.c b/ruby.c index 2242f45cfa7818..05e248a1ecb8d5 100644 --- a/ruby.c +++ b/ruby.c @@ -244,14 +244,12 @@ static const char esc_none[] = ""; #define USAGE_INDENT " " /* macro for concatenation */ static void -show_usage_line(const struct ruby_opt_message *m, +show_usage_part(const char *str, const unsigned int namelen, + const char *str2, const unsigned int secondlen, + const char *desc, int help, int highlight, unsigned int w, int columns) { static const int indent_width = (int)rb_strlen_lit(USAGE_INDENT); - const char *str = m->str; - const char *str2 = str + m->namelen; - const char *desc = str + m->namelen + m->secondlen; - const unsigned int namelen = m->namelen - 1, secondlen = m->secondlen - 1; const char *sb = highlight ? esc_bold : esc_none; const char *se = highlight ? esc_reset : esc_none; unsigned int desclen = (unsigned int)strcspn(desc, "\n"); @@ -285,6 +283,27 @@ show_usage_line(const struct ruby_opt_message *m, } } +static void +show_usage_line(const struct ruby_opt_message *m, + int help, int highlight, unsigned int w, int columns) +{ + const char *str = m->str; + const unsigned int namelen = m->namelen, secondlen = m->secondlen; + const char *desc = str + namelen + secondlen; + show_usage_part(str, namelen - 1, str + namelen, secondlen - 1, desc, + help, highlight, w, columns); +} + +void +ruby_show_usage_line(const char *name, const char *secondary, const char *description, + int help, int highlight, unsigned int w, int columns) +{ + unsigned int namelen = (unsigned int)strlen(name); + unsigned int secondlen = (secondary ? (unsigned int)strlen(secondary) : 0); + show_usage_part(name, namelen, secondary, secondlen, + description, help, highlight, w, columns); +} + static void usage(const char *name, int help, int highlight, int columns) { From ff0b763cf23ece75acb5ed13f15afa26cca2c553 Mon Sep 17 00:00:00 2001 From: Takashi Kokubun Date: Wed, 4 Oct 2023 09:30:26 -0700 Subject: [PATCH 3/5] YJIT: Move help descriptions to options.rs --- ruby.c | 15 +-------------- yjit.h | 1 + yjit/src/options.rs | 28 +++++++++++++++++++++++++++- 3 files changed, 29 insertions(+), 15 deletions(-) diff --git a/ruby.c b/ruby.c index 05e248a1ecb8d5..8020d0da11d7da 100644 --- a/ruby.c +++ b/ruby.c @@ -391,18 +391,6 @@ usage(const char *name, int help, int highlight, int columns) M("experimental", "", "experimental features"), M("performance", "", "performance issues"), }; -#if USE_YJIT - static const struct ruby_opt_message yjit_options[] = { - M("--yjit-stats", "", "Enable collecting YJIT statistics"), - M("--yjit-trace-exits", "", "Record Ruby source location when exiting from generated code"), - M("--yjit-trace-exits-sample-rate", "", "Trace exit locations only every Nth occurrence"), - M("--yjit-exec-mem-size=num", "", "Size of executable memory block in MiB (default: 128)"), - M("--yjit-call-threshold=num", "", "Number of calls to trigger JIT (default: 30)"), - M("--yjit-cold-threshold=num", "", "Global call after which ISEQs not compiled (default: 200K)"), - M("--yjit-max-versions=num", "", "Maximum number of versions per basic block (default: 4)"), - M("--yjit-greedy-versioning", "", "Greedy versioning mode (default: disabled)"), - }; -#endif #if USE_RJIT extern const struct ruby_opt_message rb_rjit_option_messages[]; #endif @@ -434,8 +422,7 @@ usage(const char *name, int help, int highlight, int columns) SHOW(warn_categories[i]); #if USE_YJIT printf("%s""YJIT options:%s\n", sb, se); - for (i = 0; i < numberof(yjit_options); ++i) - SHOW(yjit_options[i]); + rb_yjit_print_options(help, highlight, w, columns); #endif #if USE_RJIT printf("%s""RJIT options (experimental):%s\n", sb, se); diff --git a/yjit.h b/yjit.h index f43e90c7ab98c4..effca96dfe2557 100644 --- a/yjit.h +++ b/yjit.h @@ -43,6 +43,7 @@ void rb_yjit_iseq_free(void *payload); void rb_yjit_before_ractor_spawn(void); void rb_yjit_constant_ic_update(const rb_iseq_t *const iseq, IC ic, unsigned insn_idx); void rb_yjit_tracing_invalidate_all(void); +void rb_yjit_print_options(int help, int highlight, unsigned int w, int columns); #else // !USE_YJIT diff --git a/yjit/src/options.rs b/yjit/src/options.rs index b09c827cfd971a..b0e8f0b5ee1474 100644 --- a/yjit/src/options.rs +++ b/yjit/src/options.rs @@ -1,5 +1,6 @@ -use std::ffi::CStr; +use std::{ffi::{CStr, CString}, ptr::null}; use crate::backend::current::TEMP_REGS; +use std::os::raw::{c_char, c_int, c_uint}; // Command-line options #[derive(Clone, PartialEq, Eq, Debug)] @@ -79,6 +80,17 @@ pub static mut OPTIONS: Options = Options { dump_iseq_disasm: None, }; +static YJIT_OPTIONS: [(&str, &str); 8] = [ + ("--yjit-stats", "Enable collecting YJIT statistics"), + ("--yjit-trace-exits", "Record Ruby source location when exiting from generated code"), + ("--yjit-trace-exits-sample-rate", "Trace exit locations only every Nth occurrence"), + ("--yjit-exec-mem-size=num", "Size of executable memory block in MiB (default: 128)"), + ("--yjit-call-threshold=num", "Number of calls to trigger JIT (default: 30)"), + ("--yjit-cold-threshold=num", "Global call after which ISEQs not compiled (default: 200K)"), + ("--yjit-max-versions=num", "Maximum number of versions per basic block (default: 4)"), + ("--yjit-greedy-versioning", "Greedy versioning mode (default: disabled)"), +]; + #[derive(Clone, PartialEq, Eq, Debug)] pub enum DumpDisasm { // Dump to stdout @@ -231,3 +243,17 @@ pub fn parse_option(str_ptr: *const std::os::raw::c_char) -> Option<()> { // Option successfully parsed return Some(()); } + +/// Print YJIT options for `ruby --help`. +#[no_mangle] +pub extern "C" fn rb_yjit_print_options(help: c_int, highlight: c_int, w: c_uint, columns: c_int) { + for &(name, description) in YJIT_OPTIONS.iter() { + extern "C" { + fn ruby_show_usage_line(name: *const c_char, secondary: *const c_char, description: *const c_char, + help: c_int, highlight: c_int, w: c_uint, columns: c_int); + } + let name = CString::new(name).unwrap(); + let description = CString::new(description).unwrap(); + unsafe { ruby_show_usage_line(name.as_ptr(), null(), description.as_ptr(), help, highlight, w, columns) } + } +} From 2132f3793f0a6ca63e286f4cdf7fc3e89caf9237 Mon Sep 17 00:00:00 2001 From: Takashi Kokubun Date: Wed, 4 Oct 2023 09:34:06 -0700 Subject: [PATCH 4/5] Make the function names consistent --- ruby.c | 2 +- yjit.h | 2 +- yjit/src/options.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ruby.c b/ruby.c index 8020d0da11d7da..64120e21f38120 100644 --- a/ruby.c +++ b/ruby.c @@ -422,7 +422,7 @@ usage(const char *name, int help, int highlight, int columns) SHOW(warn_categories[i]); #if USE_YJIT printf("%s""YJIT options:%s\n", sb, se); - rb_yjit_print_options(help, highlight, w, columns); + rb_yjit_show_usage(help, highlight, w, columns); #endif #if USE_RJIT printf("%s""RJIT options (experimental):%s\n", sb, se); diff --git a/yjit.h b/yjit.h index effca96dfe2557..2c7a735da71195 100644 --- a/yjit.h +++ b/yjit.h @@ -43,7 +43,7 @@ void rb_yjit_iseq_free(void *payload); void rb_yjit_before_ractor_spawn(void); void rb_yjit_constant_ic_update(const rb_iseq_t *const iseq, IC ic, unsigned insn_idx); void rb_yjit_tracing_invalidate_all(void); -void rb_yjit_print_options(int help, int highlight, unsigned int w, int columns); +void rb_yjit_show_usage(int help, int highlight, unsigned int w, int columns); #else // !USE_YJIT diff --git a/yjit/src/options.rs b/yjit/src/options.rs index b0e8f0b5ee1474..f25e7f3618bb50 100644 --- a/yjit/src/options.rs +++ b/yjit/src/options.rs @@ -246,7 +246,7 @@ pub fn parse_option(str_ptr: *const std::os::raw::c_char) -> Option<()> { /// Print YJIT options for `ruby --help`. #[no_mangle] -pub extern "C" fn rb_yjit_print_options(help: c_int, highlight: c_int, w: c_uint, columns: c_int) { +pub extern "C" fn rb_yjit_show_usage(help: c_int, highlight: c_int, w: c_uint, columns: c_int) { for &(name, description) in YJIT_OPTIONS.iter() { extern "C" { fn ruby_show_usage_line(name: *const c_char, secondary: *const c_char, description: *const c_char, From 7b8fb69d3d26c887ae2f462505981b506481a7ae Mon Sep 17 00:00:00 2001 From: Takashi Kokubun Date: Wed, 4 Oct 2023 09:40:08 -0700 Subject: [PATCH 5/5] Use a better variable name for w --- ruby.c | 4 ++-- yjit.h | 2 +- yjit/src/options.rs | 9 +++++---- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/ruby.c b/ruby.c index 64120e21f38120..9d9ba316cb4d97 100644 --- a/ruby.c +++ b/ruby.c @@ -296,12 +296,12 @@ show_usage_line(const struct ruby_opt_message *m, void ruby_show_usage_line(const char *name, const char *secondary, const char *description, - int help, int highlight, unsigned int w, int columns) + int help, int highlight, unsigned int width, int columns) { unsigned int namelen = (unsigned int)strlen(name); unsigned int secondlen = (secondary ? (unsigned int)strlen(secondary) : 0); show_usage_part(name, namelen, secondary, secondlen, - description, help, highlight, w, columns); + description, help, highlight, width, columns); } static void diff --git a/yjit.h b/yjit.h index 2c7a735da71195..ede2f80a112ceb 100644 --- a/yjit.h +++ b/yjit.h @@ -43,7 +43,7 @@ void rb_yjit_iseq_free(void *payload); void rb_yjit_before_ractor_spawn(void); void rb_yjit_constant_ic_update(const rb_iseq_t *const iseq, IC ic, unsigned insn_idx); void rb_yjit_tracing_invalidate_all(void); -void rb_yjit_show_usage(int help, int highlight, unsigned int w, int columns); +void rb_yjit_show_usage(int help, int highlight, unsigned int width, int columns); #else // !USE_YJIT diff --git a/yjit/src/options.rs b/yjit/src/options.rs index f25e7f3618bb50..e8d08aae50dac9 100644 --- a/yjit/src/options.rs +++ b/yjit/src/options.rs @@ -244,16 +244,17 @@ pub fn parse_option(str_ptr: *const std::os::raw::c_char) -> Option<()> { return Some(()); } -/// Print YJIT options for `ruby --help`. +/// Print YJIT options for `ruby --help`. `width` is width of option parts, and +/// `columns` is indent width of descriptions. #[no_mangle] -pub extern "C" fn rb_yjit_show_usage(help: c_int, highlight: c_int, w: c_uint, columns: c_int) { +pub extern "C" fn rb_yjit_show_usage(help: c_int, highlight: c_int, width: c_uint, columns: c_int) { for &(name, description) in YJIT_OPTIONS.iter() { extern "C" { fn ruby_show_usage_line(name: *const c_char, secondary: *const c_char, description: *const c_char, - help: c_int, highlight: c_int, w: c_uint, columns: c_int); + help: c_int, highlight: c_int, width: c_uint, columns: c_int); } let name = CString::new(name).unwrap(); let description = CString::new(description).unwrap(); - unsafe { ruby_show_usage_line(name.as_ptr(), null(), description.as_ptr(), help, highlight, w, columns) } + unsafe { ruby_show_usage_line(name.as_ptr(), null(), description.as_ptr(), help, highlight, width, columns) } } }