-
Notifications
You must be signed in to change notification settings - Fork 18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Negative values fix #57
Merged
Merged
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
ffa9e54
potential fix for issue: https://github.com/joular/joularjx/issues/7
kifetew b9b98e5
fix for issue #7, changed version in pom
kifetew 0615d74
fixing the problem of negative values
kifetew f03b85b
removing log statement
kifetew 2cdaa0c
undoing unlrelated changes
kifetew 2eaa890
clean up
kifetew File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,8 +14,10 @@ | |
import java.lang.management.ThreadMXBean; | ||
import java.util.ArrayList; | ||
import java.util.HashMap; | ||
import java.util.Iterator; | ||
import java.util.List; | ||
import java.util.Map; | ||
import java.util.Map.Entry; | ||
import java.util.function.ObjDoubleConsumer; | ||
import java.util.function.Predicate; | ||
import java.util.logging.Level; | ||
|
@@ -106,7 +108,14 @@ public void run() { | |
|
||
double energyAfter = cpu.getCurrentPower(cpuLoad); | ||
double cpuEnergy = energyAfter - energyBefore; | ||
|
||
|
||
// if cpuEnergy is negative, skip this cycle. | ||
// this happens when energy counter is reset during program execution | ||
if (cpuEnergy < 0) { | ||
logger.info("Negative energy delta detected, skipping this cycle: " + cpuEnergy); | ||
continue; | ||
} | ||
|
||
// Calculate CPU energy consumption of the process of the JVM all its apps | ||
double processEnergy = calculateProcessCpuEnergy(cpuLoad, processCpuLoad, cpuEnergy); | ||
|
||
|
@@ -117,9 +126,11 @@ public void run() { | |
// CPU energy for JVM process | ||
// CPU energy for all processes | ||
// We need to calculate energy for each thread | ||
long totalThreadsCpuTime = updateThreadsCpuTime(methodsStats, threadsCpuTime); | ||
var threadCpuTimePercentages = getThreadsCpuTimePercentage(threadsCpuTime, totalThreadsCpuTime, processEnergy); | ||
// long totalThreadsCpuTime = updateThreadsCpuTime(methodsStats, threadsCpuTime); | ||
// var threadCpuTimePercentages = getThreadsCpuTimePercentage(threadsCpuTime, totalThreadsCpuTime, processEnergy); | ||
|
||
var threadCpuTimePercentages = getThreadsCpuTimePercentage(methodsStats, threadsCpuTime, processEnergy); | ||
|
||
updateMethodsConsumedEnergy(methodsStats, threadCpuTimePercentages, status::addMethodConsumedEnergy, Scope.ALL); | ||
updateMethodsConsumedEnergy(methodsStatsFiltered, threadCpuTimePercentages, status::addFilteredMethodConsumedEnergy, Scope.FILTERED); | ||
|
||
|
@@ -162,7 +173,7 @@ public void run() { | |
} | ||
} | ||
|
||
/** | ||
/** | ||
* Performs the sampling step. Collects a set of stack traces for each thread. | ||
* The sampling step is performed multiple time at the frequecy of SAMPLE_RATE_MILLSECONDS, for the duration of SAMPLE_TIME_MILLISECONDS | ||
* @return for each Thread, a List of it's the stack traces | ||
|
@@ -247,47 +258,91 @@ private Map<Thread, Map<CallTree, Integer>> extractCallTreesStats(Map<Thread, Li | |
return stats; | ||
} | ||
|
||
// /** | ||
// * Updates the CPU times for each Thread. | ||
// * @param methodsStats a map of method occurences for each thread | ||
// * @param threadsCpuTime a map of CPU time per PID | ||
// * @return the total CPU time used by all the monitored threads | ||
// */ | ||
// private long updateThreadsCpuTime(Map<Thread, Map<String, Integer>> methodsStats, Map<Long, Long> threadsCpuTime) { | ||
// long totalThreadsCpuTime = 0; | ||
// for (var entry : methodsStats.entrySet()) { | ||
// // the call could return a value of -1, which means thread not found or otherwise some other problem in reading cpu time for the PID | ||
// long threadCpuTime = threadBean.getThreadCpuTime(entry.getKey().getId()); | ||
// | ||
// threadCpuTime *= entry.getValue().values().stream().mapToDouble(i -> i).sum() / sampleIterations; | ||
// | ||
// // If thread already monitored, then calculate CPU time since last time | ||
// // Fix for issue #7 (https://github.com/joular/joularjx/issues/7) | ||
// // If the last thread cpu time is <= 0, don't subtract, simply take the previous value | ||
// threadCpuTime = threadsCpuTime.merge(entry.getKey().getId(), threadCpuTime, | ||
// (present, newValue) -> newValue - present); | ||
// | ||
// totalThreadsCpuTime += threadCpuTime; | ||
// } | ||
// return totalThreadsCpuTime; | ||
// } | ||
// | ||
// /** | ||
// * Returns for each thread (PID) it's percentage of CPU time used | ||
// * @param threadsCpuTime a map of CPU time per PID | ||
// * @param totalThreadsCpuTime the CPU time used by all the monitored threads | ||
// * @param processEnergy the energy consumed by the process | ||
// * @return for each PID, the percentage of CPU time used by the associated thread | ||
// */ | ||
// private Map<Long, Double> getThreadsCpuTimePercentage(Map<Long, Long> threadsCpuTime, | ||
// long totalThreadsCpuTime, | ||
// double processEnergy) { | ||
// Map<Long, Double> threadsPower = new HashMap<>(); | ||
// for (var entry : threadsCpuTime.entrySet()) { | ||
// double percentageCpuTime = (entry.getValue() * 100.0) / totalThreadsCpuTime; | ||
// double threadPower = processEnergy * (percentageCpuTime / 100.0); | ||
// threadsPower.put(entry.getKey(), threadPower); | ||
// } | ||
// return threadsPower; | ||
// } | ||
|
||
|
||
/** | ||
* Updates the CPU times for each Thread. | ||
* @param methodsStats a map of method occurences for each thread | ||
* @param threadsCpuTime a map of CPU time per PID | ||
* @return the total CPU time used by all the monitored threads | ||
*/ | ||
private long updateThreadsCpuTime(Map<Thread, Map<String, Integer>> methodsStats, Map<Long, Long> threadsCpuTime) { | ||
long totalThreadsCpuTime = 0; | ||
for (var entry : methodsStats.entrySet()) { | ||
long threadCpuTime = threadBean.getThreadCpuTime(entry.getKey().getId()); | ||
|
||
threadCpuTime *= entry.getValue().values().stream().mapToDouble(i -> i).sum() / sampleIterations; | ||
|
||
// If thread already monitored, then calculate CPU time since last time | ||
threadCpuTime = threadsCpuTime.merge(entry.getKey().getId(), threadCpuTime, | ||
(present, newValue) -> newValue - present); | ||
|
||
totalThreadsCpuTime += threadCpuTime; | ||
} | ||
return totalThreadsCpuTime; | ||
} | ||
|
||
/** | ||
* Returns for each thread (PID) it's percentage of CPU time used | ||
* @param threadsCpuTime a map of CPU time per PID | ||
* @param totalThreadsCpuTime the CPU time used by all the monitored threads | ||
* @param processEnergy the energy consumed by the process | ||
* @return for each PID, the percentage of CPU time used by the associated thread | ||
* | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you add a description of the method in the javadoc? As you merged two methods together, you can just reuse both descriptions into one. |
||
* @param methodsStats a map of method occurrences for each thread | ||
* @param threadsCpuTime a map of CPU time per PID, contains the cpu time for each tread, resulting from the last call to getThreadCpuTime(threadId) | ||
* @param processEnergy the energy consumed by the process in the last monitoring period | ||
* @return for each PID, the percentage of energy used by the associated thread | ||
*/ | ||
private Map<Long, Double> getThreadsCpuTimePercentage(Map<Long, Long> threadsCpuTime, | ||
long totalThreadsCpuTime, | ||
double processEnergy) { | ||
Map<Long, Double> threadsPower = new HashMap<>(); | ||
for (var entry : threadsCpuTime.entrySet()) { | ||
double percentageCpuTime = (entry.getValue() * 100.0) / totalThreadsCpuTime; | ||
double threadPower = processEnergy * (percentageCpuTime / 100.0); | ||
threadsPower.put(entry.getKey(), threadPower); | ||
} | ||
return threadsPower; | ||
} | ||
|
||
private Map<Long, Double> getThreadsCpuTimePercentage(Map<Thread, Map<String, Integer>> methodsStats, | ||
Map<Long, Long> threadsCpuTime, double processEnergy) { | ||
Map<Long, Double> threadsCpuTimePercentage = new HashMap<Long, Double>(); | ||
|
||
Map<Long, Double> actualThreadsCpuTime = new HashMap<Long, Double>(); | ||
double totalThreadsCpuTime = 0; | ||
// first compute the proportion of cpu time for each thread in the last sampling period | ||
for (Entry<Thread, Map<String, Integer>> threadEntry : methodsStats.entrySet()) { | ||
long threadId = threadEntry.getKey().getId(); | ||
long currentThreadCpuTime = threadBean.getThreadCpuTime(threadId); | ||
long previousThreadCpuTime = threadsCpuTime.getOrDefault(threadId, 0l); | ||
if (currentThreadCpuTime < 0) { // thread has quit | ||
// TODO ignore last sampling period?? | ||
adelnoureddine marked this conversation as resolved.
Show resolved
Hide resolved
|
||
currentThreadCpuTime = previousThreadCpuTime + 1; // FIXME assume interval of 1 nanosecond, ok?? | ||
logger.info("Thread CPU time negative, taking previous time + 1: " + currentThreadCpuTime + " for thread: " + threadId); | ||
} | ||
|
||
threadsCpuTime.put(threadId, currentThreadCpuTime); | ||
long delta = currentThreadCpuTime - previousThreadCpuTime; | ||
double adjustedThreadCpuTime = delta * threadEntry.getValue().values().stream().mapToDouble(i -> i).sum() / sampleIterations; | ||
totalThreadsCpuTime += adjustedThreadCpuTime; | ||
actualThreadsCpuTime.put(threadId, adjustedThreadCpuTime); | ||
} | ||
|
||
// compute the proportion of total energy consumed by the thread using its proportion of cpu time in the last sampling period | ||
for (Entry<Long, Double> threadEntry : actualThreadsCpuTime.entrySet()) { | ||
double threadEnergy = totalThreadsCpuTime > 0d?threadEntry.getValue() * processEnergy / totalThreadsCpuTime : 0d; | ||
threadsCpuTimePercentage.put(threadEntry.getKey(), threadEnergy); | ||
} | ||
|
||
return threadsCpuTimePercentage; | ||
} | ||
|
||
/** | ||
* Update method's consumed energy. | ||
* @param methodsStats method's encounters statistics per Thread | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather not comment existing methods, but modify them (or remove), as it's easier to see the changes with your commit