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

gen_event: eliminate selective-receive in fetch_msg/6 #6199

Merged
merged 1 commit into from
Aug 9, 2022

Conversation

mikpe
Copy link
Contributor

@mikpe mikpe commented Aug 4, 2022

Perform a non-selective receive with timeout, and pass the
message to factored-out decode_msg/7, similar to gen_server.

One of our more heavily used gen_event servers got into a state of extreme memory consumption and slow processing a couple of times this summer. When I looked into it recently I discovered that gen_event's fetch-and-dispatch loop performs a selective receive looking for system and EXIT messages (without using the ref trick), which could explain the issues. gen_server always picks the first message, and I strongly believe gen_event should do the same.

This bug seems to have been there "for ever" -- even R6B0 has this difference between gen_event and gen_server.

Perform a non-selective receive with timeout, and pass the
message to factored-out decode_msg/7, similar to gen_server.
@github-actions
Copy link
Contributor

github-actions bot commented Aug 4, 2022

CT Test Results

       2 files       85 suites   32m 35s ⏱️
1 696 tests 1 649 ✔️ 47 💤 0
1 969 runs  1 920 ✔️ 49 💤 0

Results for commit 9074e5f.

♻️ This comment has been updated with latest results.

To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass.

See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally.

Artifacts

// Erlang/OTP Github Action Bot

@RaimoNiskanen RaimoNiskanen added team:VM Assigned to OTP team VM enhancement fix testing currently being tested, tag is used by OTP internal CI in progress labels Aug 5, 2022
@RaimoNiskanen RaimoNiskanen self-assigned this Aug 5, 2022
@RaimoNiskanen
Copy link
Contributor

Since the receive statement has got a catchall Msg your proposed change should be absolutely safe.

This was once probably just a matter of code style and nobody has happened to detect the performance bug...

@RaimoNiskanen RaimoNiskanen removed the fix label Aug 9, 2022
@RaimoNiskanen RaimoNiskanen added this to the OTP-25.1 milestone Aug 9, 2022
@RaimoNiskanen RaimoNiskanen merged commit 5010539 into erlang:maint Aug 9, 2022
@RaimoNiskanen RaimoNiskanen removed the testing currently being tested, tag is used by OTP internal CI label Aug 9, 2022
@RaimoNiskanen
Copy link
Contributor

Thank you for your pull request. Great finding!

@mikpe mikpe deleted the gen_event-selective-receive-bug branch August 9, 2022 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants