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.

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

call plugin in parallel #613

Merged
merged 2 commits into from
Sep 21, 2023
Merged

call plugin in parallel #613

merged 2 commits into from
Sep 21, 2023

Conversation

yuyang0
Copy link
Contributor

@yuyang0 yuyang0 commented Sep 21, 2023

No description provided.

@CMGS
Copy link
Contributor

CMGS commented Sep 21, 2023

ut

Comment on lines 24 to 25
mu.Lock()
defer mu.Unlock()
Copy link
Contributor

Choose a reason for hiding this comment

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

我觉得不如用个 buffer chan 传错误出来……反正长度是可控的

Copy link
Contributor Author

Choose a reason for hiding this comment

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

results还是需要锁保护

Copy link
Contributor

Choose a reason for hiding this comment

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

results 可以无锁 map,我记得 core 依赖了个haxmap 吧

Copy link
Contributor Author

Choose a reason for hiding this comment

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

可以,这个UT又很奇怪,完全不相干的地方,而且本地跑好像没问题

Copy link
Contributor

Choose a reason for hiding this comment

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

重跑一次好了

Copy link
Contributor Author

Choose a reason for hiding this comment

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

mutex删了,haxmap没法用,plugin不能作为它的key,感觉这么写更丑了

Copy link
Contributor

Choose a reason for hiding this comment

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

我看了下既然都用上了 sync.Map, 不如更进一步 sync.Map 存 error,看上去没这么丑?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

应该好了吧

@CMGS CMGS merged commit fb44089 into projecteru2:master Sep 21, 2023
3 checks passed
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