Code review needed - setFetchSize()

Openfire Openfire Dev
1

I’m currently working on org.jivesoftware.openfire.user.DefaultUserProvider.getUsers(int startIndex, int numResults) and it contains this code:

DbConnectionManager.setFetchSize(rs, startIndex + numResults);

As the calling method is interested in numResults´´ results I really wonder why the fetch size is set tostartIndex + numResults´´. There are quite a few places in this class and I’d like to change them all to

DbConnectionManager.setFetchSize(rs, numResults);

Anyone who’d like to keep the current code?

Actually I did replace ``Statement´´ with "PreparedStatement´´ but it turns out that this class can be further optimized.

2

The original code is probably like this, because it is expected that the JDBC driver will return all rows from the start of the result set (even if we will skip over the first few ones). If you want row 7 through 10, and the query will return 20 rows, you need just the first 10 rows in your buffer, to allow the cursor to be moved to the 7th position and still include all rows that you’re interested in.

In any case, these are merely hints to the driver anyway. I’d leave the code as-is, unless you have measured a significant increase in performance.

3

I wonder how good it is to set the size to “1010” when one is interested in the rows 1001-1010 of 2000 results. This may waste a lot of memory for the livetime of the result set.

JDBC drivers which do not support “relative()” (Jive: scrollResultSet()) may be slower if they need to fetch 101x10 rows over a slow TCP link.

I did not try to measure a performance difference. The queries within this class are not used too often so performance shouldn’t be an issue.

Maybe this could be a solution for everyone:

if (DbConnectionManager.isScrollResultsSupported()) {
    DbConnectionManager.setFetchSize(rs, numResults);
} else {
    DbConnectionManager.setFetchSize(rs, Math.min(1000, startIndex + numResults));
}
DbConnectionManager.scrollResultSet(rs, startIndex);
4

To be honest, whatever we come up with will have drawbacks. If anything, I’d remove de code completely and let the Driver figure it out. It will very likely do a better job than we do anyways.

I’d not spend more time on this, and just leave everything as-is. It doesn’t pay, I think.