Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: replace godirwalk with filepath.WalkDir #1058

Merged
merged 1 commit into from
Oct 4, 2024
Merged

fix: replace godirwalk with filepath.WalkDir #1058

merged 1 commit into from
Oct 4, 2024

Conversation

zerok
Copy link
Contributor

@zerok zerok commented Jun 7, 2024

This removes an external dependency and hopefully also fixes #1055.

Performance-wise the two implementations look comparable:

Old:

goos: darwin
goarch: arm64
pkg: github.com/grafana/tanka/pkg/jsonnet
BenchmarkFindImporters-8               	      85	  12308651 ns/op
BenchmarkFindImporters_StaticCases/no_files-8         	  530050	      2272 ns/op
BenchmarkFindImporters_StaticCases/invalid_file-8     	   86710	     13794 ns/op
BenchmarkFindImporters_StaticCases/project_with_no_imports-8         	    3116	    386994 ns/op
BenchmarkFindImporters_StaticCases/local_import-8                    	    3112	    384138 ns/op
BenchmarkFindImporters_StaticCases/local_import_with_relative_path-8 	    3141	    382911 ns/op
BenchmarkFindImporters_StaticCases/lib_imported_through_chain-8      	    3099	    387749 ns/op
BenchmarkFindImporters_StaticCases/vendored_lib_imported_through_chain_+_directly-8         	    3039	    383107 ns/op
BenchmarkFindImporters_StaticCases/vendored_lib_found_through_symlink-8                     	    3115	    384220 ns/op
BenchmarkFindImporters_StaticCases/text_file-8                                              	    3121	    385579 ns/op
BenchmarkFindImporters_StaticCases/relative_imported_environment-8                          	    3109	    383340 ns/op
BenchmarkFindImporters_StaticCases/relative_imported_environment_with_doubled_'..'-8        	    3148	    382611 ns/op
BenchmarkFindImporters_StaticCases/relative_imported_text_file-8                            	    3152	    382753 ns/op
BenchmarkFindImporters_StaticCases/relative_imported_text_file_with_doubled_'..'-8          	    3123	    390173 ns/op
BenchmarkFindImporters_StaticCases/vendor_override_in_env:_override_vendor_used-8           	    3093	    382785 ns/op
BenchmarkFindImporters_StaticCases/vendor_override_in_env:_global_vendor_unused-8           	    3134	    382218 ns/op
BenchmarkFindImporters_StaticCases/imported_file_in_lib_relative_to_env-8                   	    3136	    382655 ns/op
BenchmarkFindImporters_StaticCases/unused_deleted_file-8                                    	  256819	      4691 ns/op
BenchmarkFindImporters_StaticCases/deleted_local_path_that_is_still_potentially_imported-8  	  233666	      5029 ns/op
BenchmarkFindImporters_StaticCases/deleted_lib_that_is_still_potentially_imported-8         	  246162	      4877 ns/op
BenchmarkFindImporters_StaticCases/deleted_vendor_that_is_still_potentially_imported-8      	  246010	      4889 ns/op
BenchmarkFindImporters_StaticCases/deleted_lib_that_is_still_potentially_imported,_relative_path_from_root-8         	  251054	      4800 ns/op
BenchmarkFindImporters_StaticCases/deleted_dir_in_environment-8                                                      	  243165	      4966 ns/op
BenchmarkFindImporters_StaticCases/imports_through_a_main_file_are_followed-8                                        	    3120	    382977 ns/op
BenchmarkGetSnippetHash/all-imported-from-main-8                                                                     	      69	  16845759 ns/op
BenchmarkGetSnippetHash/deeply-nested-8                                                                              	      69	  16665107 ns/op
PASS
ok  	github.com/grafana/tanka/pkg/jsonnet	34.298s

New:

goos: darwin
goarch: arm64
pkg: github.com/grafana/tanka/pkg/jsonnet
BenchmarkFindImporters-8               	     102	  11829940 ns/op
BenchmarkFindImporters_StaticCases/no_files-8         	  529087	      2282 ns/op
BenchmarkFindImporters_StaticCases/invalid_file-8     	   86583	     13791 ns/op
BenchmarkFindImporters_StaticCases/project_with_no_imports-8         	    3076	    384882 ns/op
BenchmarkFindImporters_StaticCases/local_import-8                    	    3126	    428205 ns/op
BenchmarkFindImporters_StaticCases/local_import_with_relative_path-8 	    3118	    384546 ns/op
BenchmarkFindImporters_StaticCases/lib_imported_through_chain-8      	    3039	    385703 ns/op
BenchmarkFindImporters_StaticCases/vendored_lib_imported_through_chain_+_directly-8         	    3118	    386329 ns/op
BenchmarkFindImporters_StaticCases/vendored_lib_found_through_symlink-8                     	    3043	    384431 ns/op
BenchmarkFindImporters_StaticCases/text_file-8                                              	    3096	    387576 ns/op
BenchmarkFindImporters_StaticCases/relative_imported_environment-8                          	    3094	    385090 ns/op
BenchmarkFindImporters_StaticCases/relative_imported_environment_with_doubled_'..'-8        	    3106	    384936 ns/op
BenchmarkFindImporters_StaticCases/relative_imported_text_file-8                            	    3135	    384335 ns/op
BenchmarkFindImporters_StaticCases/relative_imported_text_file_with_doubled_'..'-8          	    3120	    384643 ns/op
BenchmarkFindImporters_StaticCases/vendor_override_in_env:_override_vendor_used-8           	    3136	    385253 ns/op
BenchmarkFindImporters_StaticCases/vendor_override_in_env:_global_vendor_unused-8           	    3140	    387898 ns/op
BenchmarkFindImporters_StaticCases/imported_file_in_lib_relative_to_env-8                   	    3121	    385021 ns/op
BenchmarkFindImporters_StaticCases/unused_deleted_file-8                                    	  255633	      4729 ns/op
BenchmarkFindImporters_StaticCases/deleted_local_path_that_is_still_potentially_imported-8  	  238418	      5022 ns/op
BenchmarkFindImporters_StaticCases/deleted_lib_that_is_still_potentially_imported-8         	  246826	      4930 ns/op
BenchmarkFindImporters_StaticCases/deleted_vendor_that_is_still_potentially_imported-8      	  245030	      4912 ns/op
BenchmarkFindImporters_StaticCases/deleted_lib_that_is_still_potentially_imported,_relative_path_from_root-8         	  250392	      4834 ns/op
BenchmarkFindImporters_StaticCases/deleted_dir_in_environment-8                                                      	  233756	      4973 ns/op
BenchmarkFindImporters_StaticCases/imports_through_a_main_file_are_followed-8                                        	    3100	    386400 ns/op
BenchmarkGetSnippetHash/all-imported-from-main-8                                                                     	      69	  16851776 ns/op
BenchmarkGetSnippetHash/deeply-nested-8                                                                              	      70	  16859413 ns/op
PASS
ok  	github.com/grafana/tanka/pkg/jsonnet	35.749s

@github-actions GitHub Actions
Copy link

github-actions bot commented Jun 7, 2024

PR Preview Action v1.4.7
🚀 Deployed preview to https://grafana.github.io/tanka/pr-preview/pr-1058/
on branch gh-pages at 2024-06-07 11:05 UTC

@zerok zerok mentioned this pull request Jun 11, 2024
Closed
@zerok zerok changed the title fix: Replace godirwalk with filepath.WalkDir fix: replace godirwalk with filepath.WalkDir Aug 1, 2024
@zerok zerok requested review from a team and julienduchesne October 4, 2024 06:51
@zerok zerok force-pushed the zerok/filepath-walkdir branch 2 times, most recently from e411a54 to e8e183b Compare October 4, 2024 10:56
@zerok zerok enabled auto-merge (squash) October 4, 2024 11:12
@zerok zerok merged commit aac92fd into main Oct 4, 2024
9 checks passed
@zerok zerok deleted the zerok/filepath-walkdir branch October 4, 2024 11:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Elfo404 Elfo404 Elfo404 approved these changes

@julienduchesne julienduchesne Awaiting requested review from julienduchesne

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

tk export broken on Windows
2 participants
@zerok @Elfo404