SQLConnect - A library for connecting Objective-C/Swift applications to Microsoft SQL Server

Question 1

The complete SQLConnect library can be found here.

The file from which the following method belongs can be found here.

At some point I may or may not open questions for reviewing other aspects of this library, but I intend to keep the questions relatively small. This is the largest method in the entire library and is responsible for all the hard work.

Once the user has successfully connected to the database, they call this method in order to execute a SQL command on the database. This happens in a background thread. The thread waits for response from the server, then parses the results into Objective-C objects.

The structure of the results look like this:

Results is an array of tables.
Each table is an array of rows.
Each row is a dictionary, with the key being the column name and the value being the value in that column.

I'm curious to any ways the code can be more efficient from a speed perspective. In practice, I've found that any query that selects less than about 1000 rows can produce the results without the user really noticing. Once we venture into queries over 1000 rows, the time to finish all this works starts to be noticeable by the user.

Now obviously, a slow query will run slow. But I can run a query that will select 5000 rows in SQL Management Studio that returns relatively quickly, while the same query here takes noticeably longer. I don't have any specific examples (nor do I know of a publicly available SQL Server for you to test against), but my testing has shown that there is definitely more time spent turning the results into usable Objective-C objects than there is time spent executing the query and getting the results.

Here's the code:

- (void)execute:(NSString *)statement {
 void(^cleanUp)(SQLColumn *, SQLColumn *, int numCols) = ^(SQLColumn *pcol, SQLColumn *columns, int numCols) {
 for (pcol = columns; pcol - columns < numCols; ++pcol) {
 free(pcol->dataBuffer);
 }
 free(columns);
 };
 // work on background queue
 [self.workerQueue addOperationWithBlock:^{
 // set execute timeout
 dbsettime(self.executeTimeout);
 // prepare the SQL statement
 dbcmd(_connection, [statement UTF8String]);
 // attempt to execute the statement
 if (dbsqlexec(_connection) == FAIL) {
 NSError *error = [NSError errorWithDomain:kSQL_ExecutionError
 code:SQL_ExecutionError
 userInfo:nil];
 [self executionFailure:error];
 return;
 }
 // create an array to contain the result tables
 NSMutableArray *result = [NSMutableArray array];
 SQLColumn *columns;
 SQLColumn *pcol;
 int statusCode;
 // loop through each table
 while ((statusCode = dbresults(_connection)) != NO_MORE_RESULTS) {
 int numCols;
 int row_code;
 // create an array to contain the result rows for this table
 NSMutableArray *table = [NSMutableArray array];
 // get number of columns
 numCols = dbnumcols(_connection);
 // allocate C-style array of COL structs
 columns = calloc(numCols, sizeof(SQLColumn));
 // handle allocation error
 if (columns == NULL) {
 NSError *error = [NSError errorWithDomain:kSQL_ColumnStructFailedToInitialize
 code:SQL_ColumnsStructFailedToInitialize
 userInfo:nil];
 [self executionFailure:error];
 return;
 }
 // bind the column info
 for (pcol = columns; pcol - columns < numCols; ++pcol) {
 // get current column number
 int c = (int)(pcol - columns + 1);
 // get column metadata
 pcol->columnName = dbcolname(_connection, c);
 pcol->dataType = dbcoltype(_connection, c);
 pcol->dataSize = dbcollen(_connection, c);
 // if the column is varchar, we want defined size.
 // otherwise, we want max size when represented as a string
 if (pcol->dataType != SYBCHAR) {
 pcol->dataSize = dbwillconvert(pcol->dataType, SYBCHAR);
 }
 // allocate memory in the current pcol struct for a buffer
 pcol->dataBuffer = calloc(1, pcol->dataSize + 1);
 // handle allocation error
 if (pcol->dataBuffer == NULL) {
 NSError *error = [NSError errorWithDomain:kSQL_BufferFailedToAllocate
 code:SQL_BufferFailedToAllocate
 userInfo:nil];
 [self executionFailure:error];
 // clean up
 cleanUp(pcol, columns, numCols);
 return;
 }
 // bind column name
 statusCode = dbbind(_connection, c, NTBSTRINGBIND, pcol->dataSize + 1, (BYTE*)pcol->dataBuffer);
 if (statusCode == FAIL) {
 NSError *error = [NSError errorWithDomain:kSQL_ErrorBindingColumnName
 code:SQL_ErrorBindingColumnName
 userInfo:nil];
 [self executionFailure:error];
 // clean up
 cleanUp(pcol, columns, numCols);
 return;
 }
 // bind column status
 statusCode = dbnullbind(_connection, c, &pcol->columnStatus);
 if (statusCode == FAIL) {
 NSError *error = [NSError errorWithDomain:kSQL_ErrorBindingColumnStatus
 code:SQL_ErrorBindingColumnStatus
 userInfo:nil];
 [self executionFailure:error];
 // clean up
 cleanUp(pcol, columns, numCols);
 return;
 }
 }
 // done binding column info
 // loop through each row
 while ((row_code = dbnextrow(_connection)) != NO_MORE_ROWS) {
 // check row type
 switch (row_code) {
 // regular row
 case REG_ROW: {
 // create a dictionary to contain the column names and values
 NSMutableDictionary *row = [NSMutableDictionary dictionaryWithCapacity:numCols];
 // loop through each column, creating an entry in row where dictionary[columnName] = columnValue
 for (pcol = columns; pcol - columns < numCols; ++pcol) {
 NSString *columnName = [NSString stringWithUTF8String:pcol->columnName];
 id columnValue;
 // check if column has NULL value
 if (pcol->columnStatus == -1) {
 columnValue = [NSNull null];
 } else {
 // TODO: type checking
 columnValue = [NSString stringWithUTF8String:pcol->dataBuffer];
 }
 // insert the value into the dictionary
 row[columnName] = columnValue;
 }
 // add an immutable copy of the row to the table
 [table addObject:[row copy]];
 break;
 }
 // buffer full
 case BUF_FULL: {
 NSError *error = [NSError errorWithDomain:kSQL_BufferFull
 code:SQL_BufferFull
 userInfo:nil];
 [self executionFailure:error];
 // clean up
 cleanUp(pcol, columns, numCols);
 return;
 }
 // error
 case FAIL: {
 NSError *error = [NSError errorWithDomain:kSQL_RowReadError
 code:SQL_RowReadError
 userInfo:nil];
 [self executionFailure:error];
 // clean up
 cleanUp(pcol, columns, numCols);
 return;
 }
 // unknown row type
 default: {
 [self message:SQLConnectRowIgnoreMessage];
 break;
 }
 }
 }
 // clean up
 cleanUp(pcol, columns, numCols);
 // add immutable copy of table to result
 [result addObject:[table copy]];
 }
 // parsing results complete, return immutable copy of results
 [self executionSuccess:result];
 }];
}

As a note, this library is built on top of the FreeTDS API.

Question 2

Have you done any detailed profiling? My first guess is that the array appends or dictionary creations are by far the most costly thing (which you seem to have found too by your comment about turning things into ObjC objects). Dynamic arrays can involve tons of copies, so that very well may be the bottleneck. Likewise, the dictionaries and their use involves tons of allocations and copies as well. Hard numbers would give us a better hint to start from and a baseline to compare to.

Question 3

Because I'm able to know at the time of creating each dictionary how many columns are in the row, I can create the dictionaries with dictionaryWithCapacity, which means the dictionary data shouldn't ever have to be copied and moved anywhere. I'm fairly certain the problem has to do with the mutable array representing a table. It's unlikely that someone will have a large number of tables returned. But when we have to copy and move 1000 pointers to dictionaries? I imagine this is the slow down.

Question 4

I actually meant allocations and copies of the strings (though really they're not copied, they're just recreated). I suspect you're right that the array is the slow down, but depending on the underlying implementation of NSString, RC string creations is somewhere between expensive and super expensive. Best case it requires copying the C string. Worst case it requires allocating a buffer and copying the C string. Since the column names don't change, and NSString are immutable, you can reuse NSString and have C string creations instead of CR.

Question 5

@Corbin The three things that were by far taking the most time in profiler were, in this order, dbnextrow, stringWithUTF8, and whatever the underlying method that actually gets called with this syntax: row[columnName] = columnValue; (I don't remember the method name, but basically comparing the keys to make sure the columnName key isn't already in the dictionary is expensive).

Question 6

Hmm, so pulling the results from the server is the most expensive (which makes sense and can't be avoided/optimized) followed by creating ObjC strings. Hmmm. What kind of percentage is it? Are the 3rd or 4th ranked things even worth focusing on, or is everything relatively close? Also, which stringWithUTF8? What's the time split over column data and column names?

Question 7

Style and Correctness

Before I get into your desired efficiency points, I'd like to first take a stab at style and correctness (both of which are minor, and my apologies if you weren't particularly looking for this type of feedback).

This is a really, really big function. I would seriously consider compartmentalizing it into a few private methods or static functions (the C kind). At the very least, pull some of your loop body or switch cases into functions.

You're not checking the return of dbcmd, and it can fail (not very likely though, of course -- just a good idea to do especially thorough error handling in any kind of library).

Declare variables in the tightest scope possible. It's confusing looking through this and wondering where in the world certain variables came from only to realize that they were set halfway up the function even though they're reset each loop iteration. (columns andpcol` are examples of this.)

Unless you're doing pointer trickery with void*, It's typically a good idea with sizeof to use the deferenced pointer instead of the type. This leaves you less likely to make typos, and if the type changes, it only has to be updated in one place. (For example, columns = calloc(numCols, sizeof(SQLColumn)) becomes columns = calloc(numCols, sizeof(*columns)).)

Why are all local variables camelCase except for row_code?

dbcollen can theoretically fail, and that's not checked (how likely this is... I would imagine not very -- just being pedantic).

// clean up immediately followed by a function named cleanUp is redundant. Considering a lot of ObjC people have no manaul resource management background, a comment might be nice, but if there's to be one, a bit more detail about motivation might be nice (and only once).

I would consider either storing an end pointer or basing off of index for your loops:

SQLColumn* columnsEnd = columns + numCols;
for (SQLColumn* pcol = columns; pcol != columnsEnd; ++pcol) { }

Or:

for (int i = 0; i < numColumns; ++i) {
 SQLColumn* pcol = columns + i;
}

// parsing results complete, return immutable copy of results
[self executionSuccess:result];

This comment seems outdated. I'm sure executionSuccess takes a NSArray rather than NSMutableArray, but there's no copying going on.

// allocate C-style array of COL structs
columns = calloc(numCols, sizeof(SQLColumn));

Same situation -- the comment doesn't seem quite right.

The FreeTDS docs are vague on this, but I believe that bailing out of reading rows in the middle of a row-fetch-loop will leave the results cursor open. This means that the server is going to have to hold on to all of its active state about the query until the next query lets it know the connection has moved on, or it times out and the sever kills it. If this is actually what happens, this could be a rather annoying little behavior for someone down the road (though it should be incredibly rare...).

For the future when you do logic more complex than converting everything to a string, you might want to store the byte for the null character directly in pcol->dataSize rather than adding it on in allocation. Types other than strings won't need a null byte. (Then again, I suppose there's merits for dataSize representing the size of the data, not the size of the buffer to store the data...)

// get current column number <-- Not really needed with a decent variable name
int columnIdx = (int)(pcol - columns + 1);

Question 8

Performance

Really there isn't much here that I think is done excessively or unnecessarily (though my knowledge of FreeTDS began about 30 minutes ago, so... yeah...). There is, however, one giant thing leaping out at me that I previously mentioned in the comments, and that's the continual creation of strings for column names.

Let's say you have 5000 rows and 5 columns. That's 15,000 NSString creations when really all you need is 5.

This comes down to pure opinion, but personally I find either of these forms much easier to follow than the mixed version.

Nothing immediately jumps out at me with regards to major efficiency problems. Everything that's done needs to be done. There are a few things I believe you can tweak about how things are done.

Just create a simple NSMutableArray* columnNames = [NSMutableArray arrayWithCapacity:numColumns]; then when looping over the columns and setting up the data, just add the name to the array. If you wanted to mix ObjC and C types, you could even have an NSString* typed name member (no idea if ARC works inside of C-style structs though -- I would guess no).

(There's also a lot of copying involved in [table copy], but that of course has its certain uses, namely that immutable objects can just have their reference count increased instead of actually having a copy done. Whether or not avoiding this copy would be worthwhile would unfortunately depend on how the data is used once it's passed on to the delegate. In other words, if the data is never copied again, refraining from creating an immutable copy would likely be faster, though potentially take more memory.)

I actually one more performance ideas as well, but it's rather out there, and you'll want to do some careful profiling/benchmarking if you go this route:

You could try compacting your column-value buffers into one buffer. In other words, you would have one buffer of size max{pcol->dataSize forall pcol} instead of individual buffers for each column. Not only does this cut down on allocations (which shouldn't really matter since they're one-off costs), it might give you a better likely hood of hitting the cache rather than constantly loading memory segments in and flushing them out a few columns later just to loop through and load them back again.

If you wanted to get really crazy, you could even have a small-ish automatically managed (i.e. stack) variable for buffering and only fall back to dynamic allocation if you absolutely have to.

Warning: this is a really low level optimization, and it's a complete shot in the dark. I have no idea what the typical memory layout of ObjC things tends to be, nor do I have a very good intuitive understanding of cache miss probability. There's a relatively high chance that you could do this change just to find that things are just as slow, or even slower...

As a closing note, I'd like to request that you post some profiling information. In particular, where is all the time being spent? In particular, which part of converting into ObjC objects? And, I'd also be interested to know if things scale linearly or not. I'm assuming not if you've asked the question, but you never know -- 5,000 rows might just be a ton of rows and that's the end of it (does SQL Management Studio actually buffer all 5,000 rows? Or is it doing pagination silently? Also, is SQL Management Studio running locally to the SQL Server instance? That would cut down on a ton of network overhead).

Question 9

I think my [row copy] and [table copy] actually need to be replaced with [NSDictionary dictionaryWithDictionary:row] and [NSArray arrayWithArray:table]. The point is that these are mutable while I'm working with them, but the objects I want to return should be immutable. Though perhaps it might be okay to just return mutable objects--should save the time of making a copy of a thousand rows.

Question 10

Hmmm yeah, especially since the copy option is still yielding a mutable copy :/. I'm curious now what the implementation is of the init with existing method is. I'm sure it still makes a copy, but the only way to avoid that is going to be to return a mutable version by a non-mutable pointer (which has its draw backs...)

Question 11

What does this mean: Also, is SQL Management Studio running locally to the SQL Server instance? exactly? There are certainly other users querying the database, but it's on a server and as far as I know, nothing is running on the server other than SQL Server.

Question 12

@nhgrif ah, I was just wondering if one connection was over the network and one wasn't. I thought maybe you were running SQL Server on a Windows dev box and connecting through it on the box for Management Studio and network from ObjC.

Question 13

After a bit of time-profiling and with some hints from Corbin's answer here, the following changes can be made to drastically increase the speed of returning parsed results:

First, only creating one NSString variable for the dictionary key per column and reusing it.

This suggestion was attempted:

If you wanted to mix ObjC and C types, you could even have an NSString* typed name member (no idea if ARC works inside of C-style structs though -- I would guess no).

But ARC strictly forbids placing Objective-C objects in structs. My only option would be to disable ARC, and maybe at some point in time ARC should be disabled for this file, but today is not that day. Instead the solution was to create an array of column names when we're binding column info:

NSMutableArray *columnNames = [NSMutableArray arrayWithCapacity:numCols];
// bind the column info
for (pcol = columns; pcol - columns < numCols; ++pcol) {
 // get current column number
 int c = (int)(pcol - columns + 1);
 // get column metadata
 pcol->columnName = dbcolname(_connection, c);
 pcol->dataType = dbcoltype(_connection, c);
 pcol->dataSize = dbcollen(_connection, c);
 [columnNames addObject:[NSString stringWithUTF8String:pcol->columnName]];

And then later, access this array where the NSString has already been created:

// loop through each column, creating an entry in row where dictionary[columnName] = columnValue
for (pcol = columns; pcol - columns < numCols; ++pcol) {
 int columnIndex = (int)(pcol - columns);
 NSString *columnName = columnNames[columnIndex];
 //NSString *columnName = [NSString stringWithUTF8String:pcol->columnName];
 id columnValue;
 // check if column has NULL value
 if (pcol->columnStatus == -1) {
 columnValue = [NSNull null];
 } else {
 // TODO: type checking
 columnValue = [NSString stringWithUTF8String:pcol->dataBuffer];
 }
 // insert the value into the dictionary
 row[columnName] = columnValue;
}

This eliminates a LOT of the execution time which was simply spent repeatedly converting the same C-string into an NSString over and over.

Next, a lot of time was spent making copies of rows/tables.

[table addObject:[row copy]];

and

[result addObject:[table copy]];

I think the original idea may have been to produce immutable results. I'm not entirely sure that calling copy actually produces immutable results, but we're certainly spending a lot of time copying. Moreover, the time profiles also suggested we were spending a decent amount of time deallocating. When we were making these copies, the original object we copied left scope and had no more strong references as soon as we ended the loop, and ARC was appropriately cleaning up the unreferenced memory. If we just don't copy the objects and return the results as mutable, we not only eliminate the time we spent copying the objects, but we also eliminate the time ARC was added to deallocate objects.

It must be noted that the consequence of this is that the results are returned as a mutable object. You must make a decision on whether speed is more important than the mutability of your results.

Corbin Corbin 10.6k2 gold badges30 silver badges51 bronze badges · Answer 1 · 2014-08-14 01:32:30Z