[ http://issues.apache.org/jira/browse/DERBY-2118?page=all ]
Mike Matrigali updated DERBY-2118: ---------------------------------- I am definitely not comfortable with the change to the setPosition() routine. If the class is using "limit" checking to pass a data structure to another module while limiting access to the structure then the checking in setPosition is not error checking - reading to "end of file" may be valid. It effectively means that in the released product the implementation provides no limit checking at all, which could be surprising to someone who called setLimit(). It may be that all uses of this interface do not need limit checking, and unfortunately derby does not have 100% code coverage so just running all the tests can't tell this (also important to run all tests SANE/INSANE with this type of change). There should be big comments and possibly additional ASSERTS with this implementation. If limits are not needed maybe all set limit interfaces should throw exceptions? If there is a need for sometimes checking and sometimes not, maybe there should be "uncheckecked" and "checked" interfaces. Again I am not sure without checking if this is the implementation that store uses, but in the case of store limits are used for non-error cases. For instance one way an object can read itself from a page is that store passes it a stream and it reads itself, and store places a limit on the stream so that it only reads the data it is "allowed" to read and does not go past it's end. Now most datatypes know their length but the current store interface does not require that - a valid implementation can stream into the page with no length and stream out until it gets end of file. This use to be the case for at least some long binary and some long char and user defined types. I am also not sure at all of all the uses of this class, and whether they need limit checking or not. Off hand I can't think of a non-error case for the setLimit case. > Change some boundary checks in ArrayInputStream to ASSERTs to improve > performance > --------------------------------------------------------------------------------- > > Key: DERBY-2118 > URL: http://issues.apache.org/jira/browse/DERBY-2118 > Project: Derby > Issue Type: Improvement > Components: Performance > Affects Versions: 10.2.1.6 > Reporter: Dyre Tjeldvoll > Priority: Trivial > Fix For: 10.3.0.0 > > Attachments: derby-2118.diff, derby-2118.stat, derby-2118.v2.diff > > > Profiling shows that a significant amount of CPU is spent doing boundary > checking in ArrayInputStream.setPosition() and ArrayInputStream.setLimit(). > These checks appear to be there to detect error conditions, so it seems more > appropriate to make them ASSERTs. Especially since they are so expensive. > DTrace analysis seems to confirm that these methods get called very > frequently: > Knut Anders Hatlen wrote the following in a message on derby-dev: > FYI, I just ran the DERBY-1961 test clients and traced them with a > DTrace script that printed how often each method was called. For the > join client, ArrayInputStream.setPosition() was the most frequently > called method (43837.7 calls/tx). For the single-record select client, > it was third (58.4 calls/tx), only beaten by Object.<init>() and > DDMWriter.ensureLength(). I think this means that setPosition() is the > engine method that is most frequently called, at least in read-mostly > transactions. ArrayInputStream.setLimit() also appeared near the top > of the list. See http://wiki.apache.org/db-derby/Derby1961MethodCalls > for the details. -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa - For more information on JIRA, see: http://www.atlassian.com/software/jira
