Skip to content

Commit

Permalink
tools: improve performance of v test-cleancode and `v fmt -inproces…
Browse files Browse the repository at this point in the history
…s -verify .` (#21450)
  • Loading branch information
spytheman committed May 7, 2024
1 parent 20f907a commit 02d123a
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 7 deletions.
39 changes: 33 additions & 6 deletions cmd/tools/vfmt.v
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@ struct FormatOptions {
is_w bool
is_diff bool
is_verbose bool
is_all bool
is_debug bool
is_noerror bool
is_verify bool // exit(1) if the file is not vfmt'ed
is_worker bool // true *only* in the worker processes. Note: workers can crash.
is_backup bool // make a `file.v.bak` copy *before* overwriting a `file.v` in place with `-w`
in_process bool // do not fork a worker process; potentially faster, but more prone to crashes for invalid files
mut:
diff_cmd string // filled in when -diff or -verify is passed
}
Expand All @@ -49,12 +49,12 @@ fn main() {
is_w: '-w' in args
is_diff: '-diff' in args
is_verbose: '-verbose' in args || '--verbose' in args
is_all: '-all' in args || '--all' in args
is_worker: '-worker' in args
is_debug: '-debug' in args
is_noerror: '-noerror' in args
is_verify: '-verify' in args
is_backup: '-backup' in args
in_process: '-inprocess' in args
}
if term_colors {
os.setenv('VCOLORS', 'always', true)
Expand Down Expand Up @@ -100,8 +100,19 @@ fn main() {
}
mut errors := 0
mut has_internal_error := false
mut prefs := setup_preferences()
for file in files {
fpath := os.real_path(file)
if foptions.is_verify && foptions.in_process {
// For a small amount of files, it is faster to process
// everything directly in the same process, single threaded,
// when vfmt is compiled with `-gc none`:
if !foptions.verify_file(prefs, fpath) {
println("${file} is not vfmt'ed")
errors++
}
continue
}
mut worker_command_array := cli_args_no_files.clone()
worker_command_array << ['-worker', util.quote_path(fpath)]
worker_cmd := worker_command_array.join(' ')
Expand Down Expand Up @@ -145,12 +156,21 @@ fn main() {
exit(ecode)
}

fn setup_preferences_and_table() (&pref.Preferences, &ast.Table) {
table := ast.new_table()
fn (foptions &FormatOptions) verify_file(prefs &pref.Preferences, fpath string) bool {
fcontent := foptions.formated_content_from_file(prefs, fpath)
content := os.read_file(fpath) or { return false }
return fcontent == content
}

fn setup_preferences() &pref.Preferences {
mut prefs := pref.new_preferences()
prefs.is_fmt = true
prefs.skip_warnings = true
return prefs, table
return prefs
}

fn setup_preferences_and_table() (&pref.Preferences, &ast.Table) {
return setup_preferences(), ast.new_table()
}

fn (foptions &FormatOptions) vlog(msg string) {
Expand All @@ -159,6 +179,13 @@ fn (foptions &FormatOptions) vlog(msg string) {
}
}

fn (foptions &FormatOptions) formated_content_from_file(prefs &pref.Preferences, file string) string {
mut table := ast.new_table()
file_ast := parser.parse_file(file, mut table, .parse_comments, prefs)
formated_content := fmt.fmt(file_ast, mut table, prefs, foptions.is_debug)
return formated_content
}

fn (foptions &FormatOptions) format_file(file string) {
foptions.vlog('vfmt2 running fmt.fmt over file: ${file}')
prefs, mut table := setup_preferences_and_table()
Expand Down Expand Up @@ -278,6 +305,6 @@ fn verror(s string) {
fn (f FormatOptions) str() string {
return
'FormatOptions{ is_l: ${f.is_l}, is_w: ${f.is_w}, is_diff: ${f.is_diff}, is_verbose: ${f.is_verbose},' +
' is_all: ${f.is_all}, is_worker: ${f.is_worker}, is_debug: ${f.is_debug}, is_noerror: ${f.is_noerror},' +
' is_worker: ${f.is_worker}, is_debug: ${f.is_debug}, is_noerror: ${f.is_noerror},' +
' is_verify: ${f.is_verify}" }'
}
2 changes: 1 addition & 1 deletion cmd/tools/vtest-cleancode.v
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ fn v_test_vetting(vargs string) ! {
fmt_cmd, fmt_args := if is_fix {
'${os.quoted_path(vexe)} fmt -w', 'fmt -w'
} else {
'${os.quoted_path(vexe)} fmt -verify', 'fmt -verify'
'${os.quoted_path(vexe)} fmt -inprocess -verify', 'fmt -inprocess -verify'
}
vfmt_list := util.find_all_v_files(vfmt_verify_list) or { return }
exceptions := util.find_all_v_files(vfmt_known_failing_exceptions) or { return }
Expand Down
5 changes: 5 additions & 0 deletions vlib/v/help/common/fmt.txt
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,11 @@ Options:
-verify Make sure the provided file is already formatted. Useful for
checking code contributions in CI for example.

-inprocess Do everything in the same process. More prone to crashes in
case of parser bugs for invalid source files, and it needs
more memory, but it is faster for thousands of files, since
it avoids the interprocess communication overhead.

Environment Variables:
VDIFF_CMD A custom tool and options that will be used for viewing the
differences between the original and the temporarily formatted
Expand Down

0 comments on commit 02d123a

Please sign in to comment.