FWD » Core Development » Database

The purpose of this issue is to find a way to reduce the number of calls to BufferManager.activeBuffers that are resulted from AbstractTempTable._hasRecords.

After a fix involving going over the buffers from AbstractTempTable.allBuffers and execute the same logic from activeBuffers, I noticed that whenever _hasRecords is called and the activeBuffers is reached, false is always returned. This means that the condition where the temp table is checked to be equal to the current object is never true. The logic was first introduced in trunk/rev.10452, since then it was improved by direct access.

Danut, the code using allBuffers is meant to check records that were not yet flushed to the database (transient) and are stored only in FWD. Thus, we need to check all buffers to see if there is a record that was not flushed yet.

• Check if we really need to check the transient records for some use-cases.
  • The code is called from TempTableBuilder.clear - is this actually considering transient records?
  • It is also called by AbstractTempTable.setCanUndo - is this a bottleneck (?) how often is _hasRecords hit from this spot?
• Iterate only the buffers over that table; not all open buffers.

The POC application calls AbstractTempTable.hasRecords and clear/setCanUndo are not called at all. I noticed that this method can be directly called from the converted code (in this case, in a for each loop).

Alexandru Lungu wrote:

This can be improved in two ways:

• Check if we really need to check the transient records for some use-cases.
  • The code is called from TempTableBuilder.clear - is this actually considering transient records?
  • It is also called by AbstractTempTable.setCanUndo - is this a bottleneck (?) how often is _hasRecords hit from this spot?

Both methods should check for transient records because they involve checking if there are any records into the table (and if any, the methods will throw an error).

I wasn't aware this could be called directly from the converted code.

Created 8196a.

I suspect the code duplicate to be quite bad architecturally.

Lets overload activeBuffers(Persistence, boolean) with activeBuffers(Persistence, boolean, Iterator<Buffer>). The first one will call by default the second one with all buffers. However, AbstractTempTable will call it only with the iterator of that table's buffers.

Please refactor.

Note that activeBuffers iterates RecordBuffer while we want to iterate Buffer instances, so those need to be casted like so: RecordBuffer next = ((BufferReference) iter.next()).buffer();. This was the main reason I did not create another method, but we can pass Iterator<RecordBuffer> as an additional parameter and prepare the Iterator beforehand.

POC regression tests passed and the results of the performance tests showed an improvement of 5.27% for the average of the last 20 runs and 0.91% for the total average.

• Total calls: number of calls to AbstractTempTable._hasRecords() / BufferManager.activeBuffers().
• Found active buffer: isActiveBuffer() returns true.
• No matching persistence / No records to associate / Records associated: as before, but the counters are split based on which method is called.

This is good news. The changes are quite safe from my POV.

This is ready for merge. Greg, this is a performance improvement we need before rebasing 7156b.

This can be closed.