Skip to content

Commit

Permalink
Fixes as per review comments by @chhsiao90
Browse files Browse the repository at this point in the history
- Fixed javadoc typos
- Better constant name
- Simplification of LeanNode aggregation code
  • Loading branch information
PhRX committed Feb 9, 2017
1 parent 54f7910 commit 9ffe330
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 49 deletions.
Expand Up @@ -31,7 +31,7 @@ public class LeanLogCollector implements LogEventListener, ProfileSource
{ {
// Class Properties // Class Properties


private static final long SECONDSTONANOS = 1000 * 1000 * 1000; private static final long SECONDS_TO_NANOS = 1000 * 1000 * 1000;


// Instance Properties // Instance Properties


Expand Down Expand Up @@ -223,7 +223,7 @@ private void updateTime(long newSeconds, long newNanos)
long secondsDiff = newSeconds - prevSeconds; long secondsDiff = newSeconds - prevSeconds;
long nanosDiff = newNanos - prevNanos; long nanosDiff = newNanos - prevNanos;


nanosSpent = (secondsDiff * SECONDSTONANOS) + nanosDiff; nanosSpent = (secondsDiff * SECONDS_TO_NANOS) + nanosDiff;
} }


prevSeconds = newSeconds; prevSeconds = newSeconds;
Expand Down
Expand Up @@ -57,24 +57,6 @@ protected LeanNode(FrameInfo frame, LeanNode parent)
childMap = new HashMap<>(); childMap = new HashMap<>();
} }


/**
* "Self constructor" for a LeanNode associated with the "bottom frame" of a stack trace, i.e. the one for which
* self time is recorded.
*
* @param frame the {@link FrameInfo} for this LeanNode
* @param nanos the self time associated with the frame
* @param parent the parent LeanNode
*/
private LeanNode(FrameInfo frame, long nanos, LeanNode parent)
{
id = ID_GENERATOR.getAndIncrement();

this.frame = frame;
data = new NumericInfo(nanos);
this.parent = parent;
childMap = new HashMap<>();
}

/** /**
* Copy constructor. * Copy constructor.
* *
Expand Down Expand Up @@ -169,29 +151,19 @@ public boolean isThreadNode()
* {@link TraceStart} following it * {@link TraceStart} following it
* @param child the child {@link FrameInfo} of the current LeanNode * @param child the child {@link FrameInfo} of the current LeanNode
* @param last a boolean indicating if the child is the last in the stack trace sample * @param last a boolean indicating if the child is the last in the stack trace sample
* @return this node * @return the aggregated child {@link LeanNode}
*/ */
public LeanNode add(long nanos, FrameInfo child, boolean last) public LeanNode add(long nanos, FrameInfo child, boolean last)
{ {
// Non-self add, which updates total time and sample count only. // Non-self add, which updates total time and sample count only.
data.add(nanos, false); data.add(nanos, false);


return childMap.compute( LeanNode childNode = childMap.computeIfAbsent(child, k -> new LeanNode(k, this));
child, if (last)
(k, v) -> {
// Create a new LeanNode if no children have been recorded for this FrameInfo childNode.addSelf(nanos);
v == null ? (last ? }
// New child is the last in stack, use "Self constructor". return childNode;
new LeanNode(child, nanos, this)
// New child is not the last in stack, use "Non-self constructor".
: new LeanNode(child, this))
// Aggregate the information into an existing child
: (last ?
// Child is last in stack, use the "self add" on existing child
v.addSelf(nanos) :
// Child is not last in stack, return existing child (its "total" numbers will be updated when its
// child is added)
v));
} }


/** /**
Expand Down
Expand Up @@ -6,7 +6,6 @@


import com.insightfullogic.honest_profiler.core.aggregation.result.Aggregation; import com.insightfullogic.honest_profiler.core.aggregation.result.Aggregation;
import com.insightfullogic.honest_profiler.core.parser.StackFrame; import com.insightfullogic.honest_profiler.core.parser.StackFrame;
import com.insightfullogic.honest_profiler.core.profiles.lean.LeanNode;


/** /**
* NumericInfo collects the four basic amounts tracked by the profiles and {@link Aggregation}s * NumericInfo collects the four basic amounts tracked by the profiles and {@link Aggregation}s
Expand Down Expand Up @@ -43,17 +42,6 @@ public NumericInfo()
this(ZERO, ZERO, 0, 0); this(ZERO, ZERO, 0, 0);
} }


/**
* Constructor for an item with self and total time known, used when creating a new {@link LeanNode}. Self and total
* time are set to the provided time, and self and total sample count are set to 1.
*
* @param selfNanos the self (and total) time
*/
public NumericInfo(long selfNanos)
{
this(BigInteger.valueOf(selfNanos), BigInteger.valueOf(selfNanos), 1, 1);
}

/** /**
* Copy constructor. * Copy constructor.
* *
Expand Down

0 comments on commit 9ffe330

Please sign in to comment.