Anonymous | Login
Project:
RSS
  
News | My View | View Issues | Roadmap | Summary

View Issue Details[ Jump to Notes ] [ Issue History ] [ Print ]
ID
0024326
TypeCategorySeverityReproducibilityDate SubmittedLast Update
defect[Openbravo ERP] A. Platformminoralways2013-07-12 14:512013-09-26 14:36
ReporteralostaleView Statuspublic 
Assigned Toalostale 
PrioritynormalResolutionfixedFixed in Version3.0MP28
StatusclosedFix in branchFixed in SCM revisiond40d287ea91a
ProjectionnoneETAnoneTarget Version
OSAnyDatabaseAnyJava version
OS VersionDatabase versionAnt version
Product VersionSCM revision 
Review Assigned Toshankarb
Web browser
ModulesCore
Regression level
Regression date
Regression introduced in release
Regression introduced by commit
Triggers an Emergency PackNo
Summary

0024326: Suboptimal parent tab getter for root tabs

DescriptionKernelUtils.getParentTab(Tab tab) method returns parent's tab for parameter tab.

In case parameter tab is a root tab it returns null.

The issue is on how root tab is calculated, instead of looking at the tab level being 0, it does the same logic as for the rest of the tabs and returns null in case nothing is found.

Even this calculation is quite fast, it is done many times (at least whenever a record is selected in grid).
Steps To Reproduce-
Proposed SolutionIn case tablevel is 0 return null without any further check
TagsPerformance
Attached Files

- Relationships [ Relation Graph ] [ Dependency Graph ]
related to defect 0024412 closedalostale Random failures in FIC 
related to defect 00244213.0MP28 closedalostale ApplicationDictionaryCachedStructures is not thread safe 

-  Notes
(0059957)
alostale (manager)
2013-07-12 14:55
edited on: 2013-07-12 14:56

 Executing this test, which calculates parent's tab of root tabs before and after the fix:

    OBCriteria<Tab> qTab = OBDal.getInstance().createCriteria(Tab.class);
    qTab.add(Restrictions.eq(Tab.PROPERTY_TABLEVEL, 0L));
    qTab.addOrderBy(Tab.PROPERTY_ID, true);
    List<String> tabIds = new ArrayList<String>();
    for (Tab tab : qTab.list()) {
      tabIds.add(tab.getId());
    }

    long t = System.currentTimeMillis();
    for (String tabId : tabIds) {
      Tab tab = OBDal.getInstance().get(Tab.class, tabId);
      KernelUtils.getInstance().getParentTab(tab);
      // clear session not to reuse cached tab, so it is more similar to actual usage
      OBDal.getInstance().getSession().clear();
    }
    System.out.println("Time: " + (System.currentTimeMillis() - t));

Before: time 1500ms
After: time 360ms

Note in both cases ~300ms is spent in DB queries before the method, so after the fix the time is ~0 vs ~1200ms
(0059958)
alostale (manager)
2013-07-12 14:58

 Regression risk none.

Executed test plan before and after the fix:

    OBCriteria<Tab> qTab = OBDal.getInstance().createCriteria(Tab.class);
    qTab.addOrderBy(Tab.PROPERTY_ID, true);
    List<String> tabIds = new ArrayList<String>();
    for (Tab tab : qTab.list()) {
      tabIds.add(tab.getId());
    }

    for (String tabId : tabIds) {
      Tab tab = OBDal.getInstance().get(Tab.class, tabId);
      System.out.println(tab + "-" + KernelUtils.getInstance().getParentTab(tab));
    }

and compared outputs, they are identical. Meaning this change doesn't modify returned parent tab for any of the ones in the system.
(0059959)
hgbot (developer)
2013-07-12 15:03

 Repository: erp/devel/pi
Changeset: b0cfc1e495435ccb1bd58f8e55bebb30bd83c634
Author: Asier Lostalé <asier.lostale <at> openbravo.com>
Date: Fri Jul 12 15:03:06 2013 +0200
URL: http://code.openbravo.com/erp/devel/pi/rev/b0cfc1e495435ccb1bd58f8e55bebb30bd83c634 [^]

fixed bug 24326: Suboptimal parent tab getter for root tabs

---
M modules/org.openbravo.client.kernel/src/org/openbravo/client/kernel/KernelUtils.java
---
(0059988)
shankarb (reporter)
2013-07-16 06:05

 Code reviewed and tested in pi changeset 436cd2fa79fc.
(0060038)
hudsonbot (developer)
2013-07-16 14:26

 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/9a5d5983399f [^]

Maturity status: Test
(0060297)
egoitz (developer)
2013-07-26 09:25

 After apply that change the following error is continuosly raised:

17ed39e2 2013-07-23 05:28:31,588 [ajp-8809-7] ERROR org.hibernate.LazyInitializationException - could not initialize proxy - no Session
(0060300)
alostale (manager)
2013-07-26 11:03

 Reopening the bug as it creates a regression: 0024412
(0060301)
hgbot (developer)
2013-07-26 11:08

 Repository: erp/devel/pi
Changeset: 338d1021bc54b3e3658c8c3c7eae605600d7bf81
Author: Asier Lostalé <asier.lostale <at> openbravo.com>
Date: Fri Jul 26 11:07:34 2013 +0200
URL: http://code.openbravo.com/erp/devel/pi/rev/338d1021bc54b3e3658c8c3c7eae605600d7bf81 [^]

fixed bug 24412, backouts fix for bug 24326

---
M modules/org.openbravo.client.kernel/src/org/openbravo/client/kernel/KernelUtils.java
---
(0060318)
hudsonbot (developer)
2013-07-26 20:32

 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/b9e43ea5aa1b [^]

Maturity status: Test
(0061370)
hgbot (developer)
2013-09-24 17:49

 Repository: erp/devel/pi
Changeset: d40d287ea91a0c156f9087d2d00fa03dd579a543
Author: Asier Lostalé <asier.lostale <at> openbravo.com>
Date: Tue Jul 30 10:36:44 2013 +0200
URL: http://code.openbravo.com/erp/devel/pi/rev/d40d287ea91a0c156f9087d2d00fa03dd579a543 [^]

fixed bug 24421, fixed bug 24326: ADCachedStructures thread safe issues

-getTab now completely initializes all tabs in the window
-it is now synchronized

Added test cases to check it

---
M modules/org.openbravo.client.application/src/org/openbravo/client/application/window/ApplicationDictionaryCachedStructures.java
M modules/org.openbravo.client.kernel/src/org/openbravo/client/kernel/KernelUtils.java
M src-test/org/openbravo/test/AllAntTaskTests.java
A src-test/org/openbravo/test/base/HiddenObjectHelper.java
A src-test/org/openbravo/test/dal/ADCachedMultiThreadTest.java
---
(0061394)
hudsonbot (developer)
2013-09-25 19:56

 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/a06bf757c7ec [^]

Maturity status: Test
(0061414)
shankarb (reporter)
2013-09-26 14:35

 Code reviewed and verified in pi changeset f497481ca351.

- Issue History
Date Modified Username Field Change
2013-07-12 14:51 alostale New Issue
2013-07-12 14:51 alostale Assigned To => alostale
2013-07-12 14:51 alostale Modules => Core
2013-07-12 14:51 alostale Triggers an Emergency Pack => No
2013-07-12 14:55 alostale Note Added: 0059957
2013-07-12 14:55 alostale Tag Attached: Performance
2013-07-12 14:56 alostale Note Edited: 0059957 View Revisions
2013-07-12 14:58 alostale Note Added: 0059958
2013-07-12 15:02 alostale Review Assigned To => shankarb
2013-07-12 15:02 alostale Summary Suboptimal parent tab getter => Suboptimal parent tab getter for root tabs
2013-07-12 15:03 hgbot Checkin
2013-07-12 15:03 hgbot Note Added: 0059959
2013-07-12 15:03 hgbot Status new => resolved
2013-07-12 15:03 hgbot Resolution open => fixed
2013-07-12 15:03 hgbot Fixed in SCM revision => http://code.openbravo.com/erp/devel/pi/rev/b0cfc1e495435ccb1bd58f8e55bebb30bd83c634 [^]
2013-07-16 06:05 shankarb Note Added: 0059988
2013-07-16 06:05 shankarb Status resolved => closed
2013-07-16 06:05 shankarb Fixed in Version => 3.0MP26
2013-07-16 14:26 hudsonbot Checkin
2013-07-16 14:26 hudsonbot Note Added: 0060038
2013-07-26 09:25 egoitz Note Added: 0060297
2013-07-26 09:25 egoitz Status closed => new
2013-07-26 09:25 egoitz Resolution fixed => open
2013-07-26 09:25 egoitz Fixed in Version 3.0MP26 =>
2013-07-26 09:29 egoitz Status new => scheduled
2013-07-26 09:37 egoitz Status scheduled => resolved
2013-07-26 09:37 egoitz Resolution open => fixed
2013-07-26 09:37 egoitz Review Assigned To shankarb => egoitz
2013-07-26 09:37 egoitz Status resolved => closed
2013-07-26 10:48 alostale Relationship added related to 0024412
2013-07-26 11:03 alostale Note Added: 0060300
2013-07-26 11:03 alostale Status closed => new
2013-07-26 11:03 alostale Resolution fixed => open
2013-07-26 11:08 hgbot Checkin
2013-07-26 11:08 hgbot Note Added: 0060301
2013-07-26 20:32 hudsonbot Checkin
2013-07-26 20:32 hudsonbot Note Added: 0060318
2013-07-29 12:27 alostale Relationship added related to 0024421
2013-09-24 17:49 hgbot Checkin
2013-09-24 17:49 hgbot Note Added: 0061370
2013-09-24 17:49 hgbot Status new => resolved
2013-09-24 17:49 hgbot Resolution open => fixed
2013-09-24 17:49 hgbot Fixed in SCM revision http://code.openbravo.com/erp/devel/pi/rev/b0cfc1e495435ccb1bd58f8e55bebb30bd83c634 [^] => http://code.openbravo.com/erp/devel/pi/rev/d40d287ea91a0c156f9087d2d00fa03dd579a543 [^]
2013-09-25 08:26 alostale Review Assigned To egoitz => shankarb
2013-09-25 19:56 hudsonbot Checkin
2013-09-25 19:57 hudsonbot Note Added: 0061394
2013-09-26 14:35 shankarb Note Added: 0061414
2013-09-26 14:35 shankarb Status resolved => closed
2013-09-26 14:36 shankarb Fixed in Version => 3.0MP28

Copyright © 2000 - 2009 MantisBT Group
Powered by Mantis Bugtracker