> On Feb. 26, 2013, 10:30 p.m., Mona Chitnis wrote:
> > not clear how daylight saving is taken care of in the range calculation. 
> > comments/modifications will help.

Nothing special is done in the range calculation for DST. Tried changing the 
logic to do

nominalInstanceCal.add(dsTimeUnit.getCalendarUnit(), -datasetFrequency);

as it is more efficient than doing

instCount[0]--;
nominalInstanceCal = (Calendar) initInstance.clone();
nominalInstanceCal.add(dsTimeUnit.getCalendarUnit(), instCount[0] * 
datasetFrequency);

Theoretically would expect them to give same results. But they differ 
especially when there is a spring DST switch. Put in a comment there, so that 
people doing optimization in future don't spend time trying it again. 


> On Feb. 26, 2013, 10:30 p.m., Mona Chitnis wrote:
> > http://svn.apache.org/repos/asf/oozie/trunk/core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java,
> >  line 905
> > <https://reviews.apache.org/r/9626/diff/1/?file=262470#file262470line905>
> >
> >     why not do
> >     for(instCount[0] = instCount[0] + end; instCount[0] >= instCount[0] + 
> > start; instCount[0]--)
> >     
> >     more readable in logic, and helps reduce 2 lines.

Thought it was more readable that way. And also this makes more array seeks and 
extra addition in each condition check. 

Actually should have removed instCount[0] and used a variable avoiding passing 
objects by reference and updating in other methods. But dint want to touch it 
at this point as getCurrentInstance() is used in lot of other places. 


- Rohini


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9626/#review17111
-----------------------------------------------------------


On Feb. 26, 2013, 8:06 p.m., Rohini Palaniswamy wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9626/
> -----------------------------------------------------------
> 
> (Updated Feb. 26, 2013, 8:06 p.m.)
> 
> 
> Review request for oozie.
> 
> 
> Description
> -------
> 
> Use a currentRange function to move common code out of the loop and avoid CPU 
> cycles spent in initial instance and current(0) calculation. 
> 
> 
> This addresses bug OOZIE-1207.
>     https://issues.apache.org/jira/browse/OOZIE-1207
> 
> 
> Diffs
> -----
> 
>   
> http://svn.apache.org/repos/asf/oozie/trunk/core/src/main/java/org/apache/oozie/command/coord/CoordCommandUtils.java
>  1450366 
>   
> http://svn.apache.org/repos/asf/oozie/trunk/core/src/main/java/org/apache/oozie/coord/CoordELFunctions.java
>  1450366 
>   
> http://svn.apache.org/repos/asf/oozie/trunk/core/src/main/java/org/apache/oozie/util/ParamChecker.java
>  1450366 
>   
> http://svn.apache.org/repos/asf/oozie/trunk/core/src/main/resources/oozie-default.xml
>  1450366 
> 
> Diff: https://reviews.apache.org/r/9626/diff/
> 
> 
> Testing
> -------
> 
> Current unit tests in CoordActionInputCheckXCommand cover the functionality. 
> No new unit tests added.
> 
> 
> Thanks,
> 
> Rohini Palaniswamy
> 
>

Reply via email to