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

Add: fbr_poll and fbr_cond_wait_wto #12

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

alpinejoe
Copy link

fbr_poll to simulate fiber friendly poll
fbr_cond_wait_wto, fbr_cond_wait with timeout

fbr_poll to simulate non-blocking poll
fbr_cond_wait_wto, fbr_cond_wait with timeout
@Lupus
Copy link
Owner

Lupus commented Jul 21, 2015

Hello,

Thank you for pull request!

I'm on vacation currently with poor Internet access. Will take a look at it
later when I'm back.
17 июля 2015 г. 18:05 пользователь "alpinejoe" [email protected]
написал:

fbr_poll to simulate fiber friendly poll

fbr_cond_wait_wto, fbr_cond_wait with timeout

You can view, comment on, or merge this pull request online at:

#12
Commit Summary

  • Add: fbr_poll and fbr_cond_wait_wto

File Changes

Patch Links:


Reply to this email directly or view it on GitHub
#12.

@Lupus
Copy link
Owner

Lupus commented Aug 16, 2015

Hey,

Sorry for the late reply.

I don't mind including fbr_cond_wait_wto(), but I'm not completely sure I understand the rationale behind fiber-friendly poll wrapper. libevfibers is based upon libev, and using some other low-level polling mechanism seems to break this principle. Can you please elaborate here?

@alpinejoe
Copy link
Author

I added this function to make it easier to port programs utilizing poll to libevfibers and use the co-operative threading model. Such programs are already built upon a event design. Converting them to fibers would be tedious.

@Lupus
Copy link
Owner

Lupus commented Aug 22, 2015

Why not convert those programs to use libev watchers directly? No conversion to fiber model required. Your approach of adding nfds libev I/O watchers and then removing them after possibly single one has fired is extremely inefficient, and I still do not understand why it would be required in first place.

Sorry, but I decline the fiber friendly poll part of your PR, as I do not see it fit the libevfibers concept. Your fbr_poll() does not use any private API's, so you can safely use this approach in your programs/libraries if you really want to.

In order to get the fbr_cond_wait_wto() merged, please arrange it as a separate commit (which makes sense anyways as it's not related to fbr_poll() changes). Thanks!

@alpinejoe
Copy link
Author

My target was to write something equivalent of pth's soft and hard syscall overrides. I understand the wrapper is inefficient, but my target was to port libraries like curl without involving libeio. Such libraries use poll as a mechanism to simply introduce a timeout to their socket calls and poll only a single descriptor.

I usually squash my commits before merging to master. I will create a commit containing fbr_cond_wait_wto and send a pull request in some time.

@Lupus
Copy link
Owner

Lupus commented Aug 24, 2015

Any adequate library, including libcurl, usually provide enough support for asynchronous operations. See this example from libcurl, it integrates libcurl with libev. We successfully used this approach and created fiber level wrapper on top, which blocks single fiber until HTTP request is completed. No poll or threads involved, works with libev and works really well.

Overriding e.g. read with fbr_read and making some software automagically work with fibers does not look like a robust solution for me. Best approach is to use support from the library to integrate with 3rd-party event loop.

What libraries aside of libcurl are you willing to port? When you say "port", you mean create fiber-friendly wrappers? Why do you consider to not involve eio? For libcurl it might be less performant than libev integration outlined above, but it depends on the expected load.

@alpinejoe
Copy link
Author

May be curl was a bad example. Libraries that do not support a asynchronous API like MySQL client would be a better one.

Overriding is a clever hack. The authors of this paper have used pth's override feature to "port" Apache and make it asynchronous. They create 100,000 "fibers". Imagine doing that in system threads! Modifying libraries for use by libevfibers may not be practical. It will create a maintenance overhead.

If I'm not mistaken, eio creates a separate thread for every sync operation. I'm trying to avoid the overhead of a system thread.

@Lupus
Copy link
Owner

Lupus commented Aug 24, 2015

Overriding is a clever hack. The authors of this paper have used pth's override feature to "port" Apache and make it asynchronous. They create 100,000 "fibers". Imagine doing that in system threads!

It's still a hack, and may void one's warranty. I did not find any reference of Apache web server in this paper, they are speaking about some "700-line test web server".

Fibers are still a source of nondeterminism. Imagine some library doing the following sequence of syscalls:

write(fd, header_buf, header_size);
write(fd, message_buf, message_size);

We can just modify it like this:

fbr_write(fctx, fd, header_buf, header_size);
fbr_write(fctx, fd, message_buf, message_size);

But this will lead us into trouble when two fibers will execute this code with single fd, as calls can be mixed up by the fiber scheduler. In order to guarantee the same behavior one has to do the following:

fbr_mutex_lock(fctx, &fd_mutex);
fbr_write(fctx, fd, header_buf, header_size);
fbr_write(fctx, fd, message_buf, message_size);
fbr_mutex_unlock(fctx, &fd_mutex);

The above cannot be achieved without modifying the source code manually.

Modifying libraries for use by libevfibers may not be practical. It will create a maintenance overhead.

This is definitely true.

If I'm not mistaken, eio creates a separate thread for every sync operation. I'm trying to avoid the overhead of a system thread.

libeio has a thread pool, creating a thread for every operation would be prohibitive. Typical overhead for a fbr_eio_custom() invocation is about 30e-6, so any blocking operation which takes for example 1e-3 is a good candidate to be wrapped into eio callback, thread interaction overhead will be pretty low. The maintenance cost for such wrapper will be low as well, given that the target library is thread safe. Thread invocation cost can also be amortized across several operations if it's feasible to buffer their inputs in memory.

@alpinejoe
Copy link
Author

It's still a hack, and may void one's warranty. I did not find any
reference of Apache web server in this paper, they are speaking about some
"700-line test web server".

Sorry, got it mixed up. libpth developers tested against the Apache web
server.

Fibers are still a source of nondeterminism.

This is true for system threads too. The only guarantee is that the first
write would be completed before it returns. Another thread's write can be
executed before the 2nd write of the first thread because threads can be
scheduled that way. Fibers are more non-deterministic because the write
itself is non-blocking; a partial buffer may be written before the write
returns.

Anyway, I see your point. I'll remove fbr_poll and send in a pull request.

@Lupus
Copy link
Owner

Lupus commented Aug 24, 2015

This is true for system threads too. The only guarantee is that the first
write would be completed before it returns. Another thread's write can be
executed before the 2nd write of the first thread because threads can be
scheduled that way.

When giving this example, I was assuming some single threaded code,
which does not account for two consecutive writes to the same socket
to be mixed up with another context.

Fibers are more non-deterministic because the write
itself is non-blocking; a partial buffer may be written before the write
returns.

That is true either. Even two singleton writes from different fibers
can theoretically get mixed up as underlying write() can be called
multiple times.

Anyway, I see your point. I'll remove fbr_poll and send in a pull request.

Thank you!

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.

3 participants