Hi Ross! On Tue, Aug 25, 2009 at 08:45:24AM -0700, Ross McFarland wrote: > i pushed up the results of some recent experimenting with > oldlibdrizzle. i realize it's old for a reason and i see the bell plan > includes it's replacement, but i wanted to share my thoughts/findings > asap in hopes of them being useful in the work towards > (new)libdrizzle:
Thanks! Just to clarify, lp:libdrizzle is the new libdrizzle, and was written from scratch starting last fall. The "oldlibdrizzle" we refer to is actually the old library based on libmysql that is only inside of a plugin inside of drizzled for the server side protocol (lp:drizzle, plugin/oldlibdrizzle). The old libdrizzle plugin is being replaced with a plugin that will use lp:libdrizzle for all server-side communication. So, lp:libdrizzle (the new libdrizzle) is what we've been building all the new tools and APIs on, such as the php-ext, python, and others. > findings/ldrizzle-proto summary: > - there's a query object that encapsulates everything you need to > work with a query, results object (which was 1/2 of what you needed) > is gone. (see example in README.txt) with oldlibdrizzle it seems like > you have to have the original params around (sql string, etc.) to see > if query has completed, which is my least favorite things about > oldlibdrizzle. You only need the query string around until it has been sent to the client (for obvious reasons). Once it is sent, you can free or reuse it. I do have a query object in there already, it's just only used for the concurrent query interface right now. I'm planning to re-factor things a bit internally to always use it and make it optional even for single queries (you can now if you really want to). > - you don't have to call things in packet order to avoid errors. > if you don't care about columns and go directly to rows the columns > are skipped for you. if you don't finish out the fields in a row, the > same happens. This is another thing I was going to add, a couple checks for major states, and if not called in proper order, either error or do what you suggest and skip things for you (like columns). With the new protocol columns will be optional at request time so this will be less of an issue. > - the passing in pointers vs having them allocated for you stuff > is inconsistent/messy, i'd prefer to always pass in allocated > pointers/stack objects and have libdrizzle do as few mallocs as > possible as the default path. doing so would encourage use of stack > variables. This is always an option, but the interface supports both. For applications that want to control all memory they can do so easily by passing valid pointers in. Having an interface that will automatically malloc things for you can be very useful though. We decided providing both would be the most robust (and this is fairly consistent for all objects). > - naming needs quite a bit of cleanup, things should be made clean > OO-ish (in naming, encapsulation, parameter order) even though it's C Agreed. I saw you mention a few, like drizzle_con_wait, which I agree should be moved to drizzle_wait since it works on a drizzle objects. I wouldn't say needs that much cleanup, but there are a couple functions bugging me too. > - everything should have a initialize and destroy that should be > called even if it's effectively a no-op. Agreed, and we have those for every object type. What do you see is missing? > - there's too much api; unless there's really strong reasons to > have two different ways to do something there probably shouldn't be > (buffered and non-buffered) if you look at how ldrizzle-proto works > everything is read on demand, assuming that is going to cause the > least amount of blocking & waiting for data when you want something > specific. having to do things one way if you use NON-BLOCKING and > another if you don't makes it complicated to start simple and build > up. So, blocking vs non-blocking has nothing to do with buffered vs unbuffered. You can use any combo of the above, one does not determine the other. The different APIs are really about buffered and non-buffered, and each has different semantics. Trying to shove this behind a single API will lead to functions that just don't make sense in a certain context. Separating the two out seemed like the most logical thing to do (they really are two APIs). I played with this initially and the API was not intuitive. > - i think the most important part of the client library is that > starting a query is non-blocking, by default. i'd bet that alone would > improve the performance of 95% of the uses of the client or at the > very least make it trivial for people to go from starting a query and > reading results to starting a query, doing some work, and then reading > the results. the biggest (related) performance gains i've seen people > achieve in the web arena come from starting the queries up front > (controller) and blocking only if the results aren't back by the time > the rendering thread (view) needs the data. the same principals apply > in many situations. Agreed, and libdrizzle was designed with just this in mind. You can do this, concurrent queries, and soon will have callback interfaces for processing query results. As far as non-blocking goes, this really is determined by how the application is driven (event vs procedural). Also, it seems the term 'non-blocking' means different things to different people. In libdrizzle, non-blocking is used to set all socket communication into non-blocking mode so a read() or write() *never* blocks, and this is trickled up to the application using the IO_WAIT return code. It is up to the calling application to notify libdrizzle when sockets are ready using either the con_wait() function or providing a set of callbacks to trigger events on the file descriptors (using libevent for example). If you want to be able to send a query and not immediately block waiting for the result, you can do this without using non-blocking I/O. You simply send the query and tell it not to wait for results immediately, the application can do something else, and then at some point come back and block on results. This is actually all blocking I/O from the caller perspective, it's just a matter of when to call those blocking functions. The biggest thing missing right now is documentation and examples of how to do just this and other common uses, and possibly making this more apparent in the API. (from the README.txt) > - all data values are transmitted as strings (not sure why this is, maybe > there's a reason for it) This is due to the current protocol, all data is sent in string form rather than native types. In the new protocol there will be an option to get string or native returns to avoid the overhead you mention. > memset() in init functions and assert(object) in all functions. It is actually inefficient and problematic with valgrind to use memset() in init functions. You end up setting memory you really don't need to clear, and when doing this, valgrind looses it's ability to check for reads of uninitialized memory. It is actually best practice now to NOT be memset()ing memory unless there are no other options. As far as the assert() checks go, they only are good for NULLs, and never for bad pointers or uninitialized memory. If you are programming mainly with preallocated objects (which is most common), this is generally a useless check that just adds a few extra instructions to each call. Problems, whether NULL, bad pointers, or bad objects, will be fairly obvious (and will cause a crash the same time an assert() would get hit). > drizzle_select_db valgrind warnings Yeah, this is a bug and I need to look into it further. Patches welcome! :) Thanks for all the valuable feedback, libdrizzle still has a bit to go and this will certainly help in refining the internals and interface! -Eric _______________________________________________ Mailing list: https://launchpad.net/~drizzle-discuss Post to : [email protected] Unsubscribe : https://launchpad.net/~drizzle-discuss More help : https://help.launchpad.net/ListHelp
