Skip to content
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

Implement our own iterator for Bucket.Entry: #2196

Merged
merged 10 commits into from
Nov 16, 2021
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ else if (treeConcept.countElements() == 1) {
mostSpecificChildren[event] = child.getPrefix();
}
catch (ConceptConfigurationException ex) {
log.error("Failed to resolve event " + bucket + "-" + event + " against concept " + treeConcept, ex);
log.error("Failed to resolve event {}-{} against concept {}", bucket, event, treeConcept, ex);
}
}

Expand Down Expand Up @@ -286,12 +286,20 @@ public static long calculateBitMask(int pathIndex, int[] mostSpecificChild) {

/**
* For every included entity, calculate min and max and store them as statistics in the CBlock.
*
* @implNote This is an unrolled implementation of {@link CDateRange#spanClosed(CDateRange)}.
*/
private static CDateRange[] calculateEntityDateIndices(Bucket bucket, int bucketSize) {
CDateRange[] spans = new CDateRange[bucketSize];
Arrays.fill(spans, CDateRange.all());
int[] mins = new int[bucketSize];
int[] maxs = new int[bucketSize];

// First initialize to an illegal state that's easy on our comparisons
Arrays.fill(mins, Integer.MAX_VALUE);
Arrays.fill(maxs, Integer.MAX_VALUE);
awildturtok marked this conversation as resolved.
Show resolved Hide resolved

Table table = bucket.getTable();


for (Column column : table.getColumns()) {
if (!column.getType().isDateCompatible()) {
continue;
Expand All @@ -307,12 +315,47 @@ private static CDateRange[] calculateEntityDateIndices(Bucket bucket, int bucket
}

CDateRange range = bucket.getAsDateRange(event, column);
spans[index] = spans[index].spanClosed(range);

if (range.hasLowerBound()) {
final int minValue = range.getMinValue();

maxs[index] = Math.max(maxs[index], minValue);
mins[index] = Math.min(mins[index], minValue);
}

if (range.hasUpperBound()) {
final int maxValue = range.getMaxValue();

maxs[index] = Math.max(maxs[index], maxValue);
mins[index] = Math.min(mins[index], maxValue);
}
}

}
}

CDateRange[] spans = new CDateRange[bucketSize];

for (int index = 0; index < bucketSize; index++) {
if (mins[index] == Integer.MAX_VALUE && maxs[index] == Integer.MIN_VALUE) {
spans[index] = CDateRange.all();
continue;
}

if (mins[index] == Integer.MAX_VALUE) {
spans[index] = CDateRange.atMost(maxs[index]);
continue;
}

if (maxs[index] == Integer.MAX_VALUE) {
awildturtok marked this conversation as resolved.
Show resolved Hide resolved
spans[index] = CDateRange.atLeast(mins[index]);
continue;
}

spans[index] = CDateRange.of(mins[index], maxs[index]);
}


Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ich wurde den Loop-Body in eine Methode packen und die dann in der oberen äußeren Loop aufrufen ohne dass dann mins und maxs existieren muss.

Ich weiß dass du hier Phase Splitting machst, aber für mich ist das eins zuweit draußen, da die beiden for-loops doch über die selbe Sache iterieren

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moment, es geht nicht nur um Phase-splitting sondern auch darum die allokationen zu sparen. Also ich müsste so oder so split phase machen

return spans;
}
}