HiRobert

Robert DiFalco wrote:

> Well, not to pick too much of a nit,

:-).

Sure, sorry for being a bit abrupt in my previous reply, it's always
worth reviewing to keep speed and efficiency in mind, and Im always
happy to adopt suggestions for a better design :-). It was just getting
late (or early ) and I was rushing a bit to get some sleep.

You are right, particular with regard to the finaliser, but the overall
impact is fairly low. More techincal bits below, first some more
justification :-).

I got this problem porting some software that run ok on other systems
(Oracle, VAX/RDB, MSSQL and Access), but would fail on Firebird. It
then took about 2 days to isolate the problem.

I have another programming example case, where the close was not done in
a finally clause, (again technically poor style but then code wasn't
mine). Exceptions would occur occasional and that meant the program
would run fine for several days then crash.

Given that others would be porting similar applications or have similar
conditions, and that it was so damn hard to find the problem. I figured
it might be an advantage if we handled the condition more robustly.

But I am happy to leave it out if David and Roman don't think its worth
adding.

I've added a few comments below,


but there is actually a noticeable

> overhead in using the "finalize" method. For very large applications it
> could have a perceivable impact on garbage collection. When any object
> has a finalize method those objects cannot move directly to the
> deallocated state. In other words, the GC has to do more work when a
> finalize method is present even if the method body does nothing.
>

I hear what you are saying about finalize methods, and your right!

In fact from a bit of testing :

Simply constructing and closing 80,000 statememts.

for (int i = 0; i < maxStmt; i++) {
Statement stmt = con.createStatement();
stmt.close();
}

with "old" code: Time elapsed: 4.072 sec
with "new" fix: Time elapsed: 6.303 sec
"old" code with finaliser in FBStatement: Time elapsed: 6.411 sec

So it took 2sec longer, and an individual connection takes 0.00005sec to
construct and destroy, and in the "new" method 0.000075sec. The extra
time seems to be all due to the finalizer added to FBStatement.

So it is the finaliser that seems to add most overhead not the weak
references.

An increase of 50% would seem a lot, but putting this in context of a
normal transaction:

for (int i = 0; i < maxStmt; i++) {
Statement stmt = con.createStatement();
stmt.executeQuery("select * from rdb$relations");
stmt.close();

Which took about 1min for 400stmts (all three versions), and would take
about 3hrs for a loop containing 80,000.

So in 3hrs of transaction we would add about 2sec of time or the
tradeoff in an individual statement which took roughly 0.07sec we were
responsible for an extra 0.000025sec.

> If you are going to use WeakReferences, it is probably a better idea to
> create your own hash-map so that you can give each WeakReference your
> own singleton ReferenceQueue for statements (since you'd want the same
> reference queue for all connections).
>

Is there any reason for prefering on queue for all connections?

> Something like:
>
> private void collectReferences()
> {
> Reference ref;
> while ( ( ref = m_referenceQueue.poll() ) != null )
> {
> final Statement statement = (Statement)ref.get();
> try
> {
> statement.close();
> }
> ....
> }
> }
>
> But of course, then you need a thread to process this. But maybe JayBird
> already has a background thread for this sort of processing. Another
> problem with the stock WeakHashMap is that every time it resizes, it
> calls WeakHashMap.expungeStaleEntries. This blocks while it goes through
> the queue and processes discarded elements, removing them from the map.
> Actually, I think ever call has this logic since it is embedded in
> WeakHashmap.getTable.
>
> So basically, I have to block and go through this logic every time I
> create or prepare a new statement or every time I close the statement or
> connection explicitly (which I'm supposed to do anyway).
>

Im not so sure it does this, certainly in my testing, on each create or
prepare of a statement it did not check for discardable entries.
(possibly on a resize of the map, but I would expect that to be a
relatively rare event).

> At least if you ditch WeakHashMap and use a single reference-queue in a
> background thread for all statements across all connection, well then I
> won't have to block on preparing statements to look for discarded
> references.

Im not sure I understand here, From my testing, WeakHashMap certainly
didn't block and check for discarded references on each new prepare or
create (in fact in the tight create Statmeent loop, it seem to maintain
a background of about 4,000 referneces which could have been discarded -
even a System.gc() would not immediately discard them all) . But
perhaps Im misunderstanding what your saying here.

I agree it's quite possible, with a reasonable bit of work to create a
more efficient weak queue system, but Im not sure if the extra work
would gain us much, and Im probably reluctant to introduce a background
thread, - but Im happy to hear more on it.


Of course, I'm never going to NOT close statements that I

> open (unless I want them open), so all of it seems kinda like a drag.

If you close, you shoudln't get any extra drag, (other than the fairly
small additional 0.000025sec per create of statment).

> Maybe not a huge drag, but still more stuff to protect people from
> themselves, stuff that ONLY benefits those who are using the API's
> incorrectly in the first place.
>

Well, yes, but it's good to handle errors and poor programming technique
robustly, I would prefer not to hear someone saying, why does my program
fall over one every couple of days on firebird, when it runs for months
at a time on Oracle or MSSQL.


Cheers

Mark