Skip to content

refactor(context): using maps.Clone - #4333

Merged
appleboy merged 1 commit into
gin-gonic:masterfrom
cuiweixie:maps.Clone_Context
Sep 21, 2025
Merged

appleboy merged 1 commit into
gin-gonic:masterfrom
cuiweixie:maps.Clone_Context

Conversation

@cuiweixie

Copy link
Copy Markdown
Contributor

https://go-review.googlesource.com/c/go/+/471400

  • With pull requests:
    • Open your pull request against master
    • Your pull request should have no more than two commits, if not you should squash them.
    • It should pass all tests in the available continuous integration systems such as GitHub Actions.
    • You should add/modify tests to cover your proposed code changes.
    • If your pull request contains a new feature, please document it on the README.

@cuiweixie

Copy link
Copy Markdown
Contributor Author

name old time/op new time/op delta
MapClone-10 65.8ms ± 7% 10.3ms ± 2% -84.30% (p=0.000 n=10+9)

name old alloc/op new alloc/op delta
MapClone-10 40.2MB ± 0% 40.5MB ± 0% +0.57% (p=0.000 n=10+9)

name old allocs/op new allocs/op delta
MapClone-10 20.0 ± 0% 23.0 ± 0% +15.00% (p=0.000 n=10+10)

@haslam-forbes

Copy link
Copy Markdown

lol sorry ignore that please (i'm not a maintainer)

@appleboy appleboy added this to the v1.11 milestone Sep 13, 2025
@appleboy
appleboy requested a review from Copilot September 13, 2025 23:38

Copilot AI 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.

Pull Request Overview

This PR refactors the Context.Copy() method to use Go's standard library maps.Clone function instead of manually copying map elements. This change simplifies the code by replacing a manual loop-based map copying approach with the built-in utility function.

  • Replaced manual map copying logic with maps.Clone
  • Added import for the maps package
  • Removed the manual make and for loop for copying the Keys map

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread context.go
Comment on lines 133 to +135
cKeys := c.Keys
cp.Keys = make(map[any]any, len(cKeys))
c.mu.RLock()
for k, v := range cKeys {
cp.Keys[k] = v
}
cp.Keys = maps.Clone(cKeys)

Copilot AI Sep 13, 2025

Copy link

Choose a reason for hiding this comment

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

The maps.Clone call should be moved inside the mutex lock section. Currently, cKeys is read outside the lock but maps.Clone is called after acquiring the lock, which could lead to a race condition if c.Keys is modified between reading cKeys and cloning it.

Copilot uses AI. Check for mistakes.
@appleboy appleboy modified the milestones: v1.11, v1.12 Sep 20, 2025
@appleboy appleboy changed the title refactor: using maps.Clone refactor(context): using maps.Clone Sep 21, 2025
@appleboy
appleboy merged commit 414de60 into gin-gonic:master Sep 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants