| ****************************************************************************** |
| * * |
| * Open Issues, Queustions, and Comments about the I/O Core * |
| * * |
| ****************************************************************************** |
| |
| |
| +--------------+ |
| | proxy/List.h | |
| +--------------+ |
| |
| DLL<class T>::in((C *c,Link<C> &l) |
| |
| what is the meaning of in() ? |
| - is c a member of this list ? |
| - is c a member of some list ? |
| |
| +-----------------------+ |
| | proxy/NetProcessor.cc | |
| +-----------------------+ |
| |
| server open |
| |
| The user interface for accepting on a port with the server() |
| call is a bit weird. We currently get notified that a new |
| connection occurred as a side-effect of the vc clone() call. |
| That's really a hack. |
| |
| Can we pass in a continuation for notification, list we do with |
| all these other calls? |
| |
| void detach_read(); |
| void detach_write(); |
| Hide VConnection::detach_read(VIO * vio = NULL) and |
| VConnection::detach_write(VIO * vio = NULL) |
| |
| +---------------------+ |
| | proxy/VConnection.h | |
| +---------------------+ |
| |
| old code |
| |
| How many of the methods in VConnection.h are really supported |
| anymore? Can we delete the vestiges of old designs so we don't |
| confuse people? |
| |
| inline void VConnection::clear_vio_queue() |
| |
| When blowing away unexecuted VIOs, if there is a passing |
| continuation, we call it with a return_state of -1. What |
| does -1 signify, and what other values are legal? Can we be |
| sure the user won't use -1? |
| |
| We need to document these APIs. |
| |
| Mutex * mutex |
| |
| What is the meaning of mutex field in class VConnection. When is it |
| used ? It looks that IOVConnection constructor creates it, but never |
| uses it. It also looks that locking is done through the mutex passed in |
| VIO. Should we remove the mutex field from class VConnection ? |
| |
| activate_io() |
| |
| Is it part of the public interface ? |
| |
| |
| +---------------+ |
| | proxy/Event.h | |
| +---------------+ |
| |
| void EventContinuation::disable() |
| |
| We disable timeout events by setting their timeout to a large |
| value. We disable quiescence events by patching them out of the |
| queue. Wouldn't it be better to have disabling work the same way |
| for both? |
| |
| For example, could disabling timeout events just move the events |
| from the active queue to the disabled queue? |
| |
| Finally, the disabled timeout value is set to 200 years past some |
| unknown time in the past. Do we have any guarantees about that |
| unknown base reference point? I'd hate the base point to be set to |
| 200 years in the past at some new OS revision, and our timeouts |
| start acting screwy. Should we change the ink_hrtime routines to |
| explicitly subtract off some startup base time, such as the time |
| when the system starts up? This way ink_hrtime would represent |
| nanoseconds since proxy startup. |
| |
| timeout_absolute |
| |
| What is this field? The time of the final timeout, after which no |
| timeouts should occur? |
| |
| +----------------+ |
| | proxy/Event.cc | |
| +----------------+ |
| |
| types of timeouts |
| |
| I need to precisely understand the different types of timeouts, |
| such as absolute, periodic, immediate. Can I timeout be both |
| absolute & periodic? The intended semantics aren't immediately |
| obvious from the code alone. |
| |
| void EventThread::execute() |
| |
| What is the purpose of the execute_hook_main() and execute_hook_init_select() |
| virtual hooks? |
| |
| We have code to check for reference counts going to zero in the |
| EventThread main loop. Is it possible to have a global memory |
| manager do this checking, or have the decrementer do this? Are |
| we doing this to make the locking easier? It seems minorly |
| annoying for every Thread to have to worry about its own |
| garbage collection. |
| |
| void EventContinuation::check_timeout() |
| |
| What does the (timeout_at >= EVENT_DISABLE_TIMEOUT) do? It |
| presumably handles disabled timeouts, but I'm not sure how. |
| |
| There appears to be an abstraction violation here, where |
| IOVConnection and EventContinuation functionality is mixed. We |
| are casting the EventContinuation into an IOVConnection, and running |
| IOVConnection methods on the continuation. There appear to be two |
| bits: read_event and write_event that enable this. |
| |
| If EventContinuation and IOVConnection are really supposed to be |
| separate, then we should probably fix up this routine. |
| |
| Also, periodic events have their timeout_at value incremented by the |
| timeout period, independent of the current wallclock time. It seems |
| possible for small timeout periods, and some processing delay, that |
| the timeout timestamps could end up shifting permanently behind the |
| real wallclock time. In other words, the scheduler can't keep up |
| with the timing requests. Is this something we should check for? |
| |
| void EventContinuation::destroy_event() |
| |
| The semantics of destruction appear to be undocumented. What happens? |
| What should the user expect? What is the free list used for? |
| |
| void EventThread::execute_hook_init() |
| |
| Is execute_hook_init() only called at thread startup time? It appears |
| that execute_hook_init() is called once each time the execute() main loop |
| is called --- is the main loop only invoked once? |
| |
| If this is only called once, why do we have to sort the timeout |
| events? It seems like no events should be set up yet, right? |
| |
| I'm confused about the control flow to this routine. |
| |
| static void sort_timeout_events(EventThread * thr) |
| |
| We patch out the list from the EventThread's timeout queue and |
| explicitly set head to NULL. We should have a method for doing |
| this, so we can change the structure of the DLL. |
| |
| EventContinuation::add_timeout() |
| |
| After scanning for the right timeout slot, why do we need this line? |
| |
| cur = prev ? prev->timeout.next : thr()->timeout_events.head; |
| |
| quiescence |
| |
| Is quiescence implemented at all? What is it intended to do? Do |
| we care about it? Should we keep the support? |
| |
| void EventThread::execute_hook_scan_events(int n) |
| |
| What is a "zombie" event? |
| |
| void EventContinuation::reset_periodic_timeout(ink_hrtime t) |
| |
| Why do we remove the timeout twice? |
| |
| +-------------+ |
| | proxy/IO.cc | |
| +-------------+ |
| |
| IO semantics |
| |
| What is the basic model implemented by IO processors? Is it the |
| "open vconnection and enqueue operations on the vconnection" model? |
| IO.cc doesn't seem to implement open(), but it does implement |
| read/write an real connections, so I'm confused. |
| |
| Are the read_event and write_event flags still used, or are they |
| superceded by active_read_vio and active_write_vio? |
| |
| void IOProcessor::accept_read |
| |
| What is accept_read? Why is the accept stuff in IO? |
| |
| void IOThread::drain_queues() |
| |
| Why two input queues? What purpose does each serve? |
| |
| void spawn_io_event(IOThread * thr, IOVConnection * e) |
| |
| How can destroyed events end up here? |
| |
| The relationship of IOVConnections and vio_queues still |
| isn't quite clear to me. |
| |
| What does activate_io() do? |
| |
| Why the close case, what do the read_vio and write_vio flags |
| signify, and how do they relate to the vio queue? |
| |
| void IOThread::execute_hook_scan_events(int n) |
| |
| Should accept handling really be handled here? |
| |
| There are a lot of jumps to Lrestart, running the entire scan |
| from the beginning. Will this ever infinite loop? Could we |
| do it more efficiently & cleanly? |
| |
| This comment is in the code: |
| |
| ??? shouldn't we update the timeout queue first ??? |
| |
| I don't understand how flush() works. |
| |
| +---------------------------+ |
| | proxy/DiskProcessor.h | |
| +---------------------------+ |
| |
| new do_io() interface |
| |
| Are open_fd(), close(), read(), write(), fseek() and lstat() part of |
| the interface ? Sould they be removed from the DiskIOVConnection |
| interface ? |
| |
| void * buf; |
| What is the meaning of this field ? Where is it used ? |
| Is it part of the public interface ? |
| |
| |
| +-----------------------------+ |
| | include/tscore/ink_hrtime.h | |
| +-----------------------------+ |
| |
| conversion routines |
| |
| I added a bunch of hrtime-->units and units-->hrtime converters |
| that we should use. |
| |