db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Mike Matrigali (JIRA)" <j...@apache.org>
Subject [jira] Updated: (DERBY-2118) Change some boundary checks in ArrayInputStream to ASSERTs to improve performance
Date Mon, 27 Nov 2006 17:05:23 GMT
     [ 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:
>            Reporter: Dyre Tjeldvoll
>            Priority: Trivial
>             Fix For:
>         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


View raw message