Project

General

Profile

Bug #7140

refactor goldencode.expr.Compiler to use Boolean.FALSE/TRUE instead of new Boolean(condition).

Added by Constantin Asofiei about 1 year ago. Updated 12 months ago.

Status:
Test
Priority:
Normal
Assignee:
Constantin Asofiei
Target version:
-
Start date:
Due date:
% Done:

100%

billable:
No
vendor_id:
GCD
case_num:
version:

History

#1 Updated by Constantin Asofiei about 1 year ago

The TRPL compiler creates a Boolean instance for each (I think) return statement where a boolean value is needed. Currently, in a large application, this creates some 6 million java.lang.Boolean instances - although is doesn't seem much (and the profiler shows this with minimal impact), we found that garbage collector and performance in general is impacted when having a high number of created references.

The change may be somewhere in Compiler.prepare, for the RETURN instruction of a rule condition, but I'm not familiar with the com.goldencode.expr code to do this change easily.

#2 Updated by Constantin Asofiei about 1 year ago

  • Status changed from New to WIP
  • Assignee set to Constantin Asofiei

#3 Updated by Constantin Asofiei about 1 year ago

  • Status changed from WIP to Review
  • % Done changed from 0 to 100

Created task branch 7140a from trunk rev 14566.

The change is in 7140a/14567.

#4 Updated by Greg Shah about 1 year ago

Eric: Please review.

#5 Updated by Eric Faulhaber about 1 year ago

Code review 7140a/14567:

It's been a long time since I looked at the expression compiler code, but the changes look good to me. Nice work jumping into this and figuring it out!

Have you retested/profiled the large application with this change applied? I am curious to know the impact.

#6 Updated by Eric Faulhaber about 1 year ago

  • Status changed from Review to Test

#7 Updated by Greg Shah about 1 year ago

If it is tested enough, you can merge it to trunk.

#8 Updated by Constantin Asofiei about 1 year ago

I'm planning to test this together with 7199c.

#9 Updated by Constantin Asofiei about 1 year ago

7140a rev 14567 was rebased from trunk rev 14572 - new rev 14573.

#10 Updated by Constantin Asofiei about 1 year ago

Eric Faulhaber wrote:

Have you retested/profiled the large application with this change applied? I am curious to know the impact.

The impact can't be easily measured with this app, as it affects only dynamic query conversion, which doesn't happen after the testing is fully 'warmed up'. Also, I've tried patching 6813a (the upPath/downPath/relativePath refactor) onto 7156a but it's kind of complex to patch.

Anyway, I have enough testing of 7140a to put this into the queue for trunk merge.

#11 Updated by Constantin Asofiei 12 months ago

7140a was merged to trunk 14582.

Also available in: Atom PDF