Openbravo Issue Tracking System - Openbravo ERP
View Issue Details
IDProjectCategoryView StatusDate SubmittedLast Update
0033187Openbravo ERPA. Platformpublic2016-06-07 20:572016-06-17 19:38
Reportershuehner 
Assigned Toshuehner 
PrioritynormalSeverityminorReproducibilityhave not tried
StatusclosedResolutionfixed 
PlatformOS5OS Version
Product Version 
Target VersionFixed in Version3.0PR16Q3 
Merge Request Status
Review Assigned Toalostale
OBNetwork customerNo
Web browser
ModulesCore
Support ticket
Regression level
Regression date
Regression introduced in release
Regression introduced by commit
Triggers an Emergency PackNo
Summary0033187: OrganizationStructureProvider loads many ADTreeNode DAL objects into memory which could be avoided.
DescriptionThe inizialize method loads many ADTreeNode dal objects into memory and temporarily stores those in instances of some internal class OrgNode.

Note: the TreeNode references are not kept around but are only temporary.

Reviewing usage of that treeNode reference shows that it is only used internally and seems not to be part of the api.

Also only 2 property of the AD_TreeNode are used at all
- getNode (ad_node_id field)
- getReportSet (parent_id).

NOTE: That strange name seems to be an error of 'synchronize terminology' associating it with some other functionally unrelated element (by name) and DAL using that for its functions name.
However that strange model class function name is part of the API now :(

Side-effect of that initialize is having all those ADTreeNode in DAL session which creates some overhead on every single flush later in the session.

Steps To Reproduce-
Proposed SolutionReplace treeNode field with 2 individual String fields in the OrgNode helper class and change references to use the new fields instead.

Return 2 strings from HQL query instead of TreeNode to not force hibernate to create those objects.

Additional Information
TagsPerformance
Relationships
related to defect 0033186 closed shuehner OBContext.setReadableOrganizations loads tons of Organization objects into DAL session 
Attached Filesdiff 33187.diff (4,874) 2016-06-07 21:15
https://issues.openbravo.com/file_download.php?file_id=9503&type=bug
Issue History
Date ModifiedUsernameFieldChange
2016-06-07 20:57shuehnerNew Issue
2016-06-07 20:57shuehnerAssigned To => platform
2016-06-07 20:57shuehnerOBNetwork customer => No
2016-06-07 20:57shuehnerModules => Core
2016-06-07 20:57shuehnerTriggers an Emergency Pack => No
2016-06-07 20:58shuehnerTag Attached: Performance
2016-06-07 21:14shuehnerRelationship addedrelated to 0033186
2016-06-07 21:15shuehnerFile Added: 33187.diff
2016-06-07 21:16shuehnerNote Added: 0087075
2016-06-13 08:42alostaleAssigned Toplatform => shuehner
2016-06-13 08:42hgbotCheckin
2016-06-13 08:42hgbotNote Added: 0087184
2016-06-13 08:42hgbotStatusnew => resolved
2016-06-13 08:42hgbotResolutionopen => fixed
2016-06-13 08:42hgbotFixed in SCM revision => http://code.openbravo.com/erp/devel/pi/rev/1f5df7e3b8b65a69a25ecf3ac13b5c6d83d96a65 [^]
2016-06-13 08:42hgbotCheckin
2016-06-13 08:42hgbotNote Added: 0087185
2016-06-13 08:43alostaleReview Assigned To => alostale
2016-06-13 08:43alostaleNote Added: 0087186
2016-06-13 08:43alostaleStatusresolved => closed
2016-06-13 08:43alostaleFixed in Version => 3.0PR16Q3
2016-06-17 19:38hudsonbotCheckin
2016-06-17 19:38hudsonbotNote Added: 0087604
2016-06-17 19:38hudsonbotCheckin
2016-06-17 19:38hudsonbotNote Added: 0087605

Notes
(0087075)
shuehner   
2016-06-07 21:16   
NOTE: Patch is only Proof Of Concept.

Essentially replace treeNode field in OrgNode by 2 separate String fields.
And then replace all usage.

Runs without failing immediately but not more testing done so far.
(0087184)
hgbot   
2016-06-13 08:42   
Repository: erp/devel/pi
Changeset: 1f5df7e3b8b65a69a25ecf3ac13b5c6d83d96a65
Author: Stefan Hühner <stefan.huehner <at> openbravo.com>
Date: Fri Jun 10 12:24:45 2016 +0200
URL: http://code.openbravo.com/erp/devel/pi/rev/1f5df7e3b8b65a69a25ecf3ac13b5c6d83d96a65 [^]

fixed bug 33187: OrganizationStructureProvider loads many ADTreeNode objects

  OrganizationStructureProvider loads in memory DAL object for each ADTreeNode of
  the organization tree. This causes initialization to be slower as well as subsequent
  flushes.

  Now only ids are loaded instead of loading full DAL node objects, this reduces
  initialization time and does not affect flushes.

---
M src/org/openbravo/dal/security/OrganizationStructureProvider.java
---
(0087185)
hgbot   
2016-06-13 08:42   
Repository: erp/devel/pi
Changeset: 21598578f8819d875bd5c6a859fa878c70608dfb
Author: Asier Lostalé <asier.lostale <at> openbravo.com>
Date: Fri Jun 10 13:43:44 2016 +0200
URL: http://code.openbravo.com/erp/devel/pi/rev/21598578f8819d875bd5c6a859fa878c70608dfb [^]

related to bug 33187: rename some mehtods/variables to make code more readable

---
M src/org/openbravo/dal/security/OrganizationStructureProvider.java
---
(0087186)
alostale   
2016-06-13 08:43   
code reviewed

tested:
- functionally it does not make any change
- flush after order loader does not contain any tree node DAL object
(0087604)
hudsonbot   
2016-06-17 19:38   
A changeset related to this issue has been promoted main and to the
Central Repository, after passing a series of tests.

Promotion changeset: https://code.openbravo.com/erp/devel/main/rev/0dc7be081b1c [^]
Maturity status: Test
(0087605)
hudsonbot   
2016-06-17 19:38   
A changeset related to this issue has been promoted main and to the
Central Repository, after passing a series of tests.

Promotion changeset: https://code.openbravo.com/erp/devel/main/rev/0dc7be081b1c [^]
Maturity status: Test