Skip to content

refactor(compatibility): hoist flag/command lists to package vars - #14190

Closed
htoyoda18 wants to merge 1 commit into
docker:mainfrom
htoyoda18:toyo/compatibility-convert-flag-vars
Closed

refactor(compatibility): hoist flag/command lists to package vars#14190
htoyoda18 wants to merge 1 commit into
docker:mainfrom
htoyoda18:toyo/compatibility-convert-flag-vars

Conversation

@htoyoda18

@htoyoda18 htoyoda18 commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

What I did

getCompletionCommands() / getBoolFlags() / getStringFlags() in
cmd/compatibility/convert.go rebuilt their slice literals on every
call, and Convert calls them once per parsed argument in its main
loop. This moves the three lists to package-level vars so they are
allocated once instead of repeatedly. No behavior change: call sites
now reference the vars directly with slices.Contains/range instead
of calling the getter functions.

Related issue

(not mandatory) A picture of a cute animal, if possible in relation to what you did
image

Signed-off-by: hiroto.toyoda <hiroto.toyoda@dena.com>
@htoyoda18
htoyoda18 requested review from a team as code owners September 6, 2026 03:12
@htoyoda18
htoyoda18 requested review from glours and ndeloof September 6, 2026 03:12
@htoyoda18
htoyoda18 force-pushed the toyo/compatibility-convert-flag-vars branch from 6aa430d to 9b8d908 Compare September 6, 2026 03:16

@glours glours left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This introduces an unconditional cost where none is needed: compatibility is imported
unconditionally in cmd/main.go, so these package-level slices now get allocated at process init for every invocation, but Convert (and these lists) only ever runs when plugin.RunningStandalone() is true. For every normal docker compose <cmd> invocation.
That's pure waste with zero payoff, whereas before, the getters simply weren't called on that path.

Not sure this micro-optimization is useful honestly

@htoyoda18

Copy link
Copy Markdown
Contributor Author

Thanks for pointing this out. I overlooked the fact that compatibility is always imported while Convert is only used in standalone mode.

Given that, this change shifts a small allocation cost from the standalone path to every invocation, so I agree that the optimization isn't worthwhile.

I'll close this PR. Thanks for the review!

@htoyoda18 htoyoda18 closed this Sep 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants