Project

General

Profile

Bug #11034

Reduce the number of Object[] created in ChangeSet

Added by Alexandru Lungu 7 months ago. Updated 6 months ago.

Status:
Test
Priority:
Normal
Assignee:
Alexandru Lungu
Target version:
-
Start date:
Due date:
% Done:

100%

billable:
No
vendor_id:
GCD
case_num:
version_reported:
version_resolved:
reviewer:
Ovidiu Maxiniuc
production:
No
env_name:
topics:

History

#1 Updated by Alexandru Lungu 7 months ago

This task is about the big number of Object[] instances created in ChangeSet. The initial capacity defined is 16 (number of expected subtransactions). Each changes level has an Object[] that is long enough to hold all columns. This means 16 * <number-of-columns> per record. For 100k records with 100 columns, that is 1k * 10k * 16 * 8 (null size) = 1.2GB. If the records are read-only, then this is completely wasted memory.

#2 Updated by Alexandru Lungu 7 months ago

  • reviewer Ovidiu Maxiniuc added

Committed 11034a/16320 fixing this issue. Please review.

#3 Updated by Alexandru Lungu 7 months ago

  • Status changed from New to Review
  • Assignee set to Alexandru Lungu
  • % Done changed from 0 to 100

#4 Updated by Ovidiu Maxiniuc 7 months ago

  • Status changed from Review to Internal Test

I think the changes in ChangeSet.java will work.

I can only recommend some non-mandatory optimisations:
  • lines 282-293. What is we move the if outside like this:
             // release the object references in the rolled back change set level for garbage collection
         if (changes[i] != null) 
         {
            for (int j = next.nextSetBit(0); j >= 0; j = next.nextSetBit(j + 1))
            {
               // null out the corresponding data element in this transaction nesting level
               changes[i][j] = null;
            }
         }

         // clear the corresponding bits for this transaction nesting level
         next.clear();
  • personally, I would rewrite the new ensureLevelCapacity() a bit, by extracting the call to ensureCapacity() in prepare(int txLevel, boolean setupLevel):
          ensureCapacity(txLevel + 1);
      if (setupLevel)
      {
         ensureLevelCapacity(txLevel);
      }
  • at line 496 we can use local width instead of baselineData.length.

#6 Updated by Alexandru Lungu 7 months ago

Assessed review in #11034-4 in 11034a/16321.
Moving ahead with testing.

#7 Updated by Alexandru Lungu 7 months ago

  • 11034a/16322 contains a typo fix
  • 11034a/16323 is a bit more invasive as it fixed the marked structure which was not in sync with changes. In other words, there were cases in which marked was null, but still used as marked.get(i) to gather the bit set on its position. Also, on reset tx support, the changes could have stayed null and marked non-null - fixed this by nullifying both marked and changes. Did extra null-checks.

With this last version, 11034a passes a large GUI application unit tests.

#8 Updated by Ovidiu Maxiniuc 7 months ago

The latest code changes are OK, from my PoV. Optionally, you may review the last H entry for typos.

#9 Updated by Alexandru Lungu 7 months ago

ChUI regression testing passed. Testing still in WIP.

#10 Updated by Alexandru Lungu 7 months ago

Another application unit tests passed. Testing still in WIP.

#11 Updated by Alexandru Lungu 6 months ago

  • Status changed from Internal Test to Merge Pending
  • ETF passed. Time difference ranged from -2.5% to +0.6% with PG.
  • Large POC tests passed. Time difference was +0.8% with MariaDB.
  • Large customer REST API tests passed. Time difference was -3%.

Preparing to merge now.

#12 Updated by Alexandru Lungu 6 months ago

  • Status changed from Merge Pending to Test

Branch 11034a was merged into trunk as rev. 16334 and archived.

Also available in: Atom PDF