Bug #8488: improve performance of WRITE-JSON - Database - Golden Code Redmine

Bug #8488

improve performance of WRITE-JSON

Added by Constantin Asofiei over 2 years ago. Updated over 2 years ago.

Status:

Test

Priority:

Normal

Assignee:

Constantin Asofiei

Target version:

Start date:

Due date:

% Done:

100%

billable:

vendor_id:

GCD

case_num:

version_reported:

version_resolved:

reviewer:

production:

env_name:

topics:

Related issues

History

#2 Updated by Constantin Asofiei over 2 years ago

Status changed from New to WIP
Assignee set to Constantin Asofiei
% Done changed from 0 to 90

Created branch 8488a from trunk rev 15077. In rev 15088 there are performance improvements for WRITE-JSON:

Avoid BDTs within internal FWD runtime.
Replaced iterators with Java 'for', where it applies.
Read the temp-table fields directly from the Record.data as Java types and not BDT.
Directly set the record in the parent buffer instead of using a findUnique

Eric, you may want to look at the changes in persist.orm.

Ovidiu, an early review is appreciated - I still need to regression test in standalone tests (especially (de)normalized temp-table extent fields in WRITE-JSON).

#3 Updated by Ovidiu Maxiniuc over 2 years ago

Constantin Asofiei wrote:

Ovidiu, an early review is appreciated - I still need to regression test in standalone tests (especially (de)normalized temp-table extent fields in WRITE-JSON).

Sure. I think the code is good. My observations are as follows:

In P2JQuery.java, you added public default boolean _getNext() which is implemented as: getNext().booleanValue(). I think we should do the other way around: the getNext() method to be 'final' (evidently cannot be) and return new logical(_getNext()). The implementing classes should use _getNext() return Java native value. You did this whole implementation in QueryWrapper, I see.
In RecordBuffer.java. We attempted to make the public API as small as possible. Does loadRecord() really need to be public?
Related to above, Property.java, the new back-reference to PropertyMeta meta is public. I know that this is probably not the final form, maybe we can find a solution which better encapsulates and grants the new member a const/final feeling?
JsonExport.java, line 1466/1467, I think they can be written as date p2jDate = new datetimetz((OffsetDateTime) d);, avoiding changing the value of d argument of the lambda. IMO, it is ugly to change the d's type (and its value, of course) that way. The same for datetime and date, below;
the XmlExport might also benefit from the same boost as JsonExport.

#4 Updated by Constantin Asofiei over 2 years ago

Ovidiu, in rev 15080 I have the review fixes. See bellow for notes:

In P2JQuery.java, you added public default boolean _getNext() which is implemented as: getNext().booleanValue(). I think we should do the other way around: the getNext() method to be 'final' (evidently cannot be) and return new logical(_getNext()). The implementing classes should use _getNext() return Java native value. You did this whole implementation in QueryWrapper, I see.

Done

In RecordBuffer.java. We attempted to make the public API as small as possible. Does loadRecord() really need to be public?

Yes, it's used from JsonExport, which is in a different package.

Related to above, Property.java, the new back-reference to PropertyMeta meta is public. I know that this is probably not the final form, maybe we can find a solution which better encapsulates and grants the new member a const/final feeling?

Fixed.

JsonExport.java, line 1466/1467, I think they can be written as date p2jDate = new datetimetz((OffsetDateTime) d);, avoiding changing the value of d argument of the lambda. IMO, it is ugly to change the d's type (and its value, of course) that way. The same for datetime and date, below;

Fixed.