****************************************************************************** * * * Open Issues, Queustions, and Comments about the I/O Core * * * ****************************************************************************** +--------------+ | proxy/List.h | +--------------+ DLL::in((C *c,Link &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 ? +---------------------------+ | libinktomi++/ink_hrtime.h | +---------------------------+ conversion routines I added a bunch of hrtime-->units and units-->hrtime converters that we should use.