Imporve performance when generating spec with external dependencies #1108

pytimer · 2022-01-21T07:34:12Z

Describe the PR

Directly use go list command instead of depth, because depth use go standard library build.Import and call the build.Import too many times, it will spent too many times when the large dependencies.

Because swag only needs to know which files are dependent, and does not need to get the level of dependencies, all dependencies can be directly through go list command, and swag not need to go through deep loop again, but if use depth, swag still need to loop too many times, so i refactor the swag get ast files code.

Relation issue

Fixes: #1032

Additional context

Added the --parseGoList flag, the default value is true default. If someone does not want to enable it, it can be set to false when init, if it's set to false, swag still use depth for dependency parser.

codecov · 2022-01-21T14:40:25Z

Codecov Report

Merging #1108 (842c7a2) into master (636f456) will increase coverage by 0.33%.
The diff coverage is 88.23%.

❗ Current head 842c7a2 differs from pull request most recent head ceb23ac. Consider uploading reports for the commit ceb23ac to get more accurate results

@@            Coverage Diff             @@
##           master    #1108      +/-   ##
==========================================
+ Coverage   95.03%   95.37%   +0.33%     
==========================================
  Files          10       11       +1     
  Lines        2557     2614      +57     
==========================================
+ Hits         2430     2493      +63     
+ Misses         69       66       -3     
+ Partials       58       55       -3

Impacted Files	Coverage Δ
parser.go	`93.69% <83.33%> (+0.82%)`	⬆️
golist.go	`90.24% <90.24%> (ø)`
gen/gen.go	`93.30% <100.00%> (+0.03%)`	⬆️
packages.go	`86.74% <100.00%> (+1.26%)`	⬆️
operation.go	`96.64% <0.00%> (+0.29%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 636f456...ceb23ac. Read the comment docs.

ubogdan · 2022-01-25T11:59:36Z

Unfortunately, I will have a busy week.

@easonlin404 can you please assist with a CR here?

magandrez · 2022-02-07T08:09:39Z

@ubogdan @easonlin404 Is there any way to contribute to this change? please, let know.

ubogdan · 2022-02-07T08:45:53Z

This implementation introduces a dependency over an external binary, the go binary, that comes with some new challenges :

The output format may change over time.
You may get slightly different output on different go versions.
You may run the generator in an environment where the GO binary it's not in PATH.
You may run the generator in an environment where the GO binary is not the same as your one to build your project.

I prefer to see this done programmatically by importing a go library that handlers go.mod and calling the specific methods.

The solution proposed by @pytimer may look like a quick fix at this time. Once the feature gets merged to master, the community will start using it, more issues will arrive, and code complexity will grow.

pytimer · 2022-02-08T08:13:30Z

@ubogdan swag currently already depends on the go binary, now if the env has not go binary, swag init failed, because init first call go list to get pkg name, the code is getPkgName.
depth call build.Default, this code also call the go binary file, if no go binary, it will return ErrRootPkgNotResolved, this pr does not newly introduce the go binary.

However, the two points 2 and 4 above @ubogdan said do need to be pay attention to. I don't know much about the output of multiple versions of go list. I only tested it on versions of go 1.16 and 1.17.
I will try to understand the output format of go list in different versions. If anyone else is familiar with it, please point it out.

ubogdan · 2022-02-08T12:08:24Z

@ubogdan swag currently already depends on the go binary,
Please don't return this as an excuse.
I explained why I wouldn't say I like the idea, but didn't refuse the implementation.
It's more simple for me to Approve and Merge and after that, I can tag you in every issue where a community member is complaining something is wrong.

ubogdan · 2022-02-08T12:10:30Z

I expect to see at least one opinion from @easonlin404 or @sdghchj regarding this matter before going forward to do a real CR and approve it.

g1eny0ung · 2022-03-07T09:18:55Z

Any updates? After my local testing, this will greatly improve the generation speed.

ubogdan · 2022-03-07T09:43:07Z

I will consider this total silence as an aproval.
@pytimer I will go forward with a code review in the next few days, please confirm you are still interested to finish this PR.

pytimer · 2022-03-07T10:12:49Z

Thanks.
I read some docs about not use binary execute command to do it recently, but unfortunately i don't find.

I try golang.org/x/tools/go/packages.Load, but it not works well.

Perhaps use go list now and find the better ways to optimize in later version, what's your recommend?

ubogdan · 2022-03-07T13:26:29Z

We will go with the current implementation. Please confirm you still intend to finish this PR so I can go with a CR.

pytimer · 2022-03-07T13:43:36Z

We will go with the current implementation. Please confirm you still intend to finish this PR so I can go with a CR.

Yes，please tell me if need I do.

ubogdan

See comments ... Also include the usage example in the README.md

ubogdan · 2022-03-08T11:57:08Z

gen/gen.go

@@ -151,6 +154,7 @@ func (g *Gen) Build(config *Config) error {
 	p.ParseVendor = config.ParseVendor
 	p.ParseDependency = config.ParseDependency
 	p.ParseInternal = config.ParseInternal
+	p.ParseGoList = config.ParseGoList



Please use the pattern above

p := swag.New(swag.SetMarkdownFileDirectory(config.MarkdownFilesDir), ...... swag.ParseUsingGoList(config.ParseGoList), }

We will clean the other ones later.

golist.go

parser.go

pytimer · 2022-03-09T02:36:41Z

@ubogdan Update the code, you can review it if you have time, thanks.

Should i squash all commits after review or now?

ubogdan

LGTM

ubogdan · 2022-03-09T14:38:35Z

@ubogdan Update the code, you can review it if you have time, thanks.

Should i squash all commits after review or now?

We are using "Squash and Merge" for all the contributions.

ubogdan · 2022-03-09T17:44:36Z

golist.go

+	// Skip cgo
+	if pkg.Name == "C" {
+		return nil
+	}


Please cover this by calling the function with a build.Package instance.

ok, i will add some testing on this Saturday, i don't have the bandwidth these days, sorry.

ubogdan · 2022-03-09T17:45:52Z

Please improve code coverage so we can merge it.

akhmadKurniawan

ok

sdghchj · 2022-03-22T07:24:08Z

I expect to see at least one opinion from @easonlin404 or @sdghchj regarding this matter before going forward to do a real CR and approve it.

As far as I see, go list will find out all its dependent packages, I am not sure whether it's more efficient in a huge project or not.

magandrez · 2022-04-08T12:00:26Z

Any ETA to land this? @ubogdan

ubogdan · 2022-04-08T12:18:04Z

@magandrez, I'm waiting for @pytimer to improve code coverage so we can merge it.

g1eny0ung · 2022-04-24T03:29:04Z

@ubogdan I wonder if it is possible to draft a new release after merging this PR? I've been looking forward to this improvement for a long time. Very grateful to everyone for this. ❤️

pytimer · 2022-04-25T03:48:30Z

@ubogdan How can i rerun github Actions and codecov?

ubogdan · 2022-04-26T17:42:45Z

Good job. Please resolve conflicts.

pytimer · 2022-04-27T01:30:32Z

@ubogdan solve conflicts, please review it, thanks.

ubogdan · 2022-04-27T14:05:41Z

Looks like code coverage is still failing :(

pytimer · 2022-04-27T16:49:00Z

Maybe i merge master branch code, add test code in the latest commit.

pytimer · 2022-05-08T01:00:53Z

Please rerun test codecov, thanks.

pytimer · 2022-05-09T16:19:16Z

@ubogdan Can you tell me how to fix codecov/patch, i don't understand where the problem, thanks.

ubogdan · 2022-05-10T06:58:56Z

Maybe you need to write unit tests this parts of the code: https://github.com/swaggo/swag/pull/1108/checks?check_run_id=6346809522? if possible...

ivanpk · 2022-05-16T18:31:19Z

Do we have any ETA regarding the required changes? What can others do to help get this in?

time ../../pytimer/swag/cmd/swag/swag init -d {directory names removed} --parseDependency --parseDepth 1 --output api/generated_docs

8.26s user 6.70s system 359% cpu 4.160 total

time swag init -d {directory names removed} --parseDependency --parseDepth 1 --output api/generated_docs

318.74s user 736.87s system 928% cpu 1:53.65 total

The performance difference is vast

ubogdan

LGTM

ubogdan · 2022-05-18T09:08:01Z

y ETA regarding the required changes

We will try to include this in the next release. I was hoping we would get better code coverage.

g1eny0ung · 2022-06-16T06:53:41Z

@ubogdan Can we release a new version? It's been a long time since this PR was merged. 😢

ubogdan · 2022-06-16T09:45:25Z

released in v1.8.3

g1eny0ung · 2022-06-16T09:48:29Z

released in v1.8.3

Many thanks!

pytimer force-pushed the golist branch 2 times, most recently from e557d3e to 8a5cb7b Compare January 25, 2022 01:37

ubogdan requested a review from easonlin404 January 25, 2022 11:58

g1eny0ung mentioned this pull request Mar 7, 2022

feat: update API requests with OpenAPI generated client chaos-mesh/chaos-mesh#2926

Merged

9 tasks

ubogdan requested changes Mar 8, 2022

View reviewed changes

ubogdan previously approved these changes Mar 9, 2022

View reviewed changes

ubogdan reviewed Mar 9, 2022

View reviewed changes

akhmadKurniawan reviewed Mar 11, 2022

View reviewed changes

pytimer dismissed ubogdan’s stale review via 0981927 March 14, 2022 09:19

pytimer force-pushed the golist branch 4 times, most recently from 56a4e31 to 9fe3da7 Compare March 30, 2022 06:54

pytimer added 2 commits April 27, 2022 09:26

Imporve performance when generating spec with external dependencies

2d4ba4a

Fix code review comments

e48e0a6

pytimer force-pushed the golist branch from 1536568 to 5355158 Compare April 27, 2022 01:28

Add go1.18 test code

ceb23ac

pytimer force-pushed the golist branch from 0f12bc4 to ceb23ac Compare April 28, 2022 13:57

ubogdan approved these changes May 18, 2022

View reviewed changes

ubogdan merged commit 5f6b402 into swaggo:master May 18, 2022

smallfish06 mentioned this pull request Jun 19, 2022

cannot find all dependencies, <nil> #1232

Open

MrDOS mentioned this pull request Jul 5, 2022

swag init treats CGo warnings as fatal #1249

Open

Imporve performance when generating spec with external dependencies #1108

Imporve performance when generating spec with external dependencies #1108

Conversation

pytimer commented Jan 21, 2022

codecov bot commented Jan 21, 2022 • edited

Codecov Report

ubogdan commented Jan 25, 2022

magandrez commented Feb 7, 2022

ubogdan commented Feb 7, 2022

pytimer commented Feb 8, 2022

ubogdan commented Feb 8, 2022

ubogdan commented Feb 8, 2022

g1eny0ung commented Mar 7, 2022

ubogdan commented Mar 7, 2022

pytimer commented Mar 7, 2022

ubogdan commented Mar 7, 2022

pytimer commented Mar 7, 2022

ubogdan left a comment

Choose a reason for hiding this comment

ubogdan Mar 8, 2022

Choose a reason for hiding this comment

pytimer commented Mar 9, 2022 • edited

ubogdan left a comment

Choose a reason for hiding this comment

ubogdan commented Mar 9, 2022

ubogdan Mar 9, 2022

Choose a reason for hiding this comment

pytimer Mar 10, 2022

Choose a reason for hiding this comment

ubogdan commented Mar 9, 2022

akhmadKurniawan left a comment

Choose a reason for hiding this comment

sdghchj commented Mar 22, 2022

magandrez commented Apr 8, 2022

ubogdan commented Apr 8, 2022

g1eny0ung commented Apr 24, 2022 • edited

pytimer commented Apr 25, 2022

ubogdan commented Apr 26, 2022

pytimer commented Apr 27, 2022

ubogdan commented Apr 27, 2022

pytimer commented Apr 27, 2022

pytimer commented May 8, 2022

pytimer commented May 9, 2022

ubogdan commented May 10, 2022 • edited

ivanpk commented May 16, 2022

ubogdan left a comment

Choose a reason for hiding this comment

ubogdan commented May 18, 2022

g1eny0ung commented Jun 16, 2022 • edited

ubogdan commented Jun 16, 2022

g1eny0ung commented Jun 16, 2022

codecov bot commented Jan 21, 2022 •

edited

pytimer commented Mar 9, 2022 •

edited

g1eny0ung commented Apr 24, 2022 •

edited

ubogdan commented May 10, 2022 •

edited

g1eny0ung commented Jun 16, 2022 •

edited