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

fix: picoev fmt #20569

Merged
merged 2 commits into from
Feb 26, 2024
Merged

fix: picoev fmt #20569

merged 2 commits into from
Feb 26, 2024

Conversation

enghitalo
Copy link
Contributor

@enghitalo enghitalo commented Jan 17, 2024

better use of OR bitwise

old generate c code

int read_events = (((((event.events & ((u32)(EPOLLIN))) != 0U ? (_const_picoev__picoev_read) : (0))) | (((event.events & ((u32)(EPOLLOUT))) != 0U ? (_const_picoev__picoev_write) : (0)))));

new generate c code

int read_events = 0;
if ((event.events & ((u32)(EPOLLIN))) != 0U) {
	read_events |= _const_picoev__picoev_read;
}
if ((event.events & ((u32)(EPOLLOUT))) != 0U) {
	read_events |= _const_picoev__picoev_write;
}

@spytheman
Copy link
Member

What is the performance impact?

@enghitalo
Copy link
Contributor Author

What is the performance impact?

I will verify ASAP

@spytheman
Copy link
Member

Any news?

@JalonSolov
Copy link
Contributor

Why not just use the POSIX poll function, which should be available on all (or most?) systems. Even Windows has a POSIX subsystem.

@enghitalo
Copy link
Contributor Author

Any news?

Sorry. I ended up getting bogged down with a lot of work during that time. Over the weekend I'll try to do some tests.

@spytheman
Copy link
Member

Why not just use the POSIX poll function, which should be available on all (or most?) systems. Even Windows has a POSIX subsystem.

TLDR: performance and scalability. Platform specific APIs like epoll and kqueue, can better handle the case of many thousands of connections, that are mostly idle. See https://stackoverflow.com/questions/5383959/why-exactly-does-epoll-scale-better-than-poll for more details, or https://en.wikipedia.org/wiki/C10k_problem .

Imho in another 5-10 years, POSIX can standardize a common API for it, but afaik it has not happened yet, and poll is not enough.

@enghitalo
Copy link
Contributor Author

enghitalo commented Feb 13, 2024

What is the performance impact?

No performance impact.

After some performance test at examples/pico/pico.v and examples/pico/raw_callback.v
using wrk with command wrk -d10s http://127.0.0.1:8080 I have noticed no result changes (margin of error around 1%).

Running 10s test @ http://127.0.0.1:8080
  2 threads and 10 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency    60.84us   21.37us 563.00us   77.52%
    Req/Sec    80.73k     8.49k  128.49k    67.16%
  1613112 requests in 10.10s, 193.84MB read
Requests/sec: 159724.51
Transfer/sec:     19.19MB

The command used to run the projetcs was ./v -prod crun <project path>

@spytheman spytheman merged commit 43fd568 into vlang:master Feb 26, 2024
42 checks passed
raw-bin pushed a commit to raw-bin/v that referenced this pull request Mar 9, 2024
raw-bin pushed a commit to raw-bin/v that referenced this pull request Mar 9, 2024
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.

None yet

3 participants