remove use of bufferevent for both plain and ssl connections#183
Merged
mattklein123 merged 3 commits intomasterfrom Nov 2, 2016
Merged
remove use of bufferevent for both plain and ssl connections#183mattklein123 merged 3 commits intomasterfrom
mattklein123 merged 3 commits intomasterfrom
Conversation
From perf investigations, a big slow down at high throughput is inside libevent. bufferevent does not use edge triggered events, so it ends up calling epoll_ctl on very write. This change implements our own version of bufferevent inside of ConnectionImpl. We need to do this for both plain and ssl connections. This is an incredibly scary change. I've already run this on a prod box for a while but will do more smoke testing before we merge and do a full canary.
Member
Author
|
@lyft/network-team |
ccaraman
reviewed
Nov 1, 2016
include/envoy/event/file_event.h
Outdated
| virtual ~FileEvent() {} | ||
|
|
||
| /** | ||
| * Active the file event explicitly for a set of events. Should be a logical OR of FileReadyType |
include/envoy/event/file_event.h
Outdated
| virtual ~FileEvent() {} | ||
|
|
||
| /** | ||
| * Active the file event explicitly for a set of events. Should be a logical OR of FileReadyType |
source/common/buffer/buffer_impl.h
Outdated
|
|
||
| #include "common/event/libevent.h" | ||
|
|
||
| // Forward decls to avoid leaking libevent headers to reset of program. |
source/common/buffer/buffer_impl.h
Outdated
| static const evbuffer_cb_func buffer_cb_; | ||
|
|
||
| Event::Libevent::BufferPtr buffer_; | ||
| Callback cb_; |
Member
There was a problem hiding this comment.
can you put comments on cb_ and buffer_cb_ ?
|
|
||
| ConnectionImpl::PostIoAction ConnectionImpl::doReadFromSocket() { | ||
| do { | ||
| int rc = read_buffer_.read(fd_, 16384); |
Member
There was a problem hiding this comment.
can you put comment on how value is chosen?
ccaraman
reviewed
Nov 1, 2016
| bool keep_reading = true; | ||
| PostIoAction action = PostIoAction::KeepOpen; | ||
| while (keep_reading) { | ||
| Buffer::RawSlice slices[2]; |
ccaraman
reviewed
Nov 1, 2016
| fd_(fd), id_(++next_global_id_) { | ||
|
|
||
| file_event_ = | ||
| dispatcher_.createFileEvent(fd, [this](uint32_t events) -> void { onFileEvent(events); }); |
ccaraman
approved these changes
Nov 2, 2016
gargnupur
added a commit
to gargnupur/envoy
that referenced
this pull request
Mar 12, 2020
This is submitting JohnPlevyak's work: https://github.com/envoyproxy/envoy-wasm/compare/master...jplevyak:get_context?expand=1 Signed-off-by: gargnupur <gargnupur@google.com>
wolfguoliang
pushed a commit
to wolfguoliang/envoy
that referenced
this pull request
Jan 23, 2021
…s/service_to_service zh-translation: docs/root/intro/deployment_types/service_to_service.rst
jpsim
pushed a commit
that referenced
this pull request
Nov 28, 2022
Resolves the following warning when building: > DEBUG: Rule 'io_bazel_rules_kotlin' indicated that a canonical reproducible form can be obtained by modifying arguments shallow_since = "1556831609 -0700" Signed-off-by: Michael Rebello <mrebello@lyft.com> Signed-off-by: JP Simard <jp@jpsim.com>
jpsim
pushed a commit
that referenced
this pull request
Nov 29, 2022
Resolves the following warning when building: > DEBUG: Rule 'io_bazel_rules_kotlin' indicated that a canonical reproducible form can be obtained by modifying arguments shallow_since = "1556831609 -0700" Signed-off-by: Michael Rebello <mrebello@lyft.com> Signed-off-by: JP Simard <jp@jpsim.com>
arminabf
pushed a commit
to arminabf/envoy
that referenced
this pull request
Jun 5, 2024
mathetake
pushed a commit
that referenced
this pull request
Mar 3, 2026
**Commit Message**: Updating the netlify.toml configurations I have validated the configurations work with Netlify here: https://celebrated-zabaione-b8a322.netlify.app/ The purpose and function of the config: - setting the `base` as `site/` which ensures netlify builds and deploys **only happen** when there's changes to the site directory, this is a special netlify config - setting up a `context.deploy-preview` so we get preview deploys for PRs when there are changes to the `site/` directory - updating headers with additional security headers to improve site --------- Signed-off-by: Erica Hughberg <erica.sundberg.90@gmail.com>
mathetake
added a commit
that referenced
this pull request
Mar 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
From perf investigations, a big slow down at high throughput is inside
libevent. bufferevent does not use edge triggered events, so it ends up
calling epoll_ctl on very write.
This change implements our own version of bufferevent inside of
ConnectionImpl. We need to do this for both plain and ssl connections.
This is an incredibly scary change. I've already run this on a prod box
for a while but will do more smoke testing before we merge and do a full
canary.