Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.enso.table.data.column.builder;

import java.lang.foreign.MemorySegment;
import java.util.BitSet;
import org.enso.table.data.column.storage.BoolStorage;
import org.enso.table.data.column.storage.ColumnBooleanStorage;
Expand All @@ -8,18 +9,31 @@
import org.enso.table.data.column.storage.type.NullType;
import org.enso.table.data.column.storage.type.StorageType;
import org.enso.table.error.ValueTypeMismatchException;
import org.enso.table.util.BitSets;
import org.enso.table.util.ImmutableBitSet;

/** A builder for boolean columns. */
final class BoolBuilder implements BuilderForBoolean, BuilderWithRetyping {
private final BitSet vals;
private final BitSet validityMap;
int size = 0;
private int size;

// ** Creates a new builder for boolean columns. Should be built via Builder.getForBoolean. */
BoolBuilder(int capacity) {
vals = new BitSet(capacity);
validityMap = new BitSet(capacity);
this(new BitSet(capacity), new BitSet(capacity), 0);
}

private BoolBuilder(BitSet vals, BitSet validityMap, int size) {
this.vals = vals;
this.validityMap = validityMap;
this.size = size;
}

static BoolBuilder fromAddress(int sizeInBits, long data, long validity) {
var bytesSize = (sizeInBits + 7) / 8;
var vals = BitSet.valueOf(MemorySegment.ofAddress(data).reinterpret(bytesSize).asByteBuffer());
var validityMap =
BitSet.valueOf(MemorySegment.ofAddress(validity).reinterpret(bytesSize).asByteBuffer());
return new BoolBuilder(vals, validityMap, sizeInBits);
}

@Override
Expand Down Expand Up @@ -72,7 +86,7 @@ public void appendBulkStorage(ColumnStorage<?> storage) {
if (storage instanceof BoolStorage boolStorage) {
// We know this is valid for a BoolStorage.
int toCopy = (int) boolStorage.getSize();
BitSets.copy(boolStorage.getValues(), vals, size, toCopy);
boolStorage.getValues().copyTo(vals, size, toCopy);
boolStorage.getValidityMap().copyTo(validityMap, size, toCopy);
size += toCopy;
} else if (storage instanceof ColumnBooleanStorage columnBooleanStorage) {
Expand All @@ -92,7 +106,13 @@ public void appendBulkStorage(ColumnStorage<?> storage) {

@Override
public ColumnStorage<Boolean> seal() {
return new BoolStorage(vals, validityMap, size, false);
return seal(null);
}

final ColumnStorage<Boolean> seal(ColumnStorage<?> other) {
var vals = new ImmutableBitSet(this.vals, size);
var validityMap = new ImmutableBitSet(this.validityMap, size);
return new BoolStorage(vals, validityMap, size, false, other);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ static <T> ColumnStorage<T> makeLocal(ColumnStorage<T> storage) {
var localType = StorageType.fromTypeCharAndSize(proxyType.typeChar(), proxyType.size());
var localStorage =
switch (localType) {
case BooleanType type -> BoolBuilder.fromAddress(size, data, validity).seal(storage);
case IntegerType type ->
LongBuilder.fromAddress(size, data, validity, type).seal(storage, type);
default -> storage;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,8 @@ private static ColumnStorage<?> applyBooleanIsIn(
// If had both true and false, then return all true when not nothing
if (flags.hadTrue && flags.hadFalse) {
var validityMap = makeValidityMap(boolStorage, checkedSize);
return new BoolStorage(new BitSet(), validityMap, checkedSize, true);
return new BoolStorage(
ImmutableBitSet.allFalse(checkedSize), validityMap, checkedSize, true, null);
}

// Only have one of true or false
Expand Down Expand Up @@ -296,7 +297,7 @@ private static ColumnStorage<?> applyBooleanIsIn(

private static ColumnStorage<?> applyBoolStorage(
boolean keepValue, BoolStorage boolStorage, int checkedSize) {
BitSet values = boolStorage.getValues();
BitSet values = boolStorage.getValues().cloneBitSet();
Copy link
Member

Choose a reason for hiding this comment

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

why do we need to clone here - I assume because it is an ImmutableBitSet now?

Copy link
Member Author

Choose a reason for hiding this comment

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

  • Yes, to get a BitSet from ImmutableBitSet one calls cloneBitSet() which does a clone/copy.
  • that way we can guarantee consistency of ImmutableBitSet instances
  • we'll see if such a change has a detrimental effect on the performance later
  • and then we can think of optimizing these additional copyings

BitSet isNothing = boolStorage.getValidityMap().cloneBitSet();
isNothing.flip(0, Math.toIntExact(boolStorage.getSize()));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import java.math.BigDecimal;
import java.math.BigInteger;
import java.util.BitSet;
import org.enso.base.polyglot.NumericConverter;
import org.enso.table.data.column.builder.Builder;
import org.enso.table.data.column.operation.BinaryOperation;
Expand Down Expand Up @@ -100,7 +99,7 @@ public ColumnStorage<?> applyZip(
public static class BooleanFillMissingOperation extends FillMissingOperation {
public static BoolStorage fillMissingBoolStorage(BoolStorage storage, boolean fillValue) {
var size = (int) storage.getSize();
var newValues = (BitSet) storage.getValues().clone();
var newValues = storage.getValues().cloneBitSet();
var isNothingMap = storage.getValidityMap().cloneBitSet();
isNothingMap.flip(0, size);
if (fillValue != storage.isNegated()) {
Expand All @@ -109,7 +108,8 @@ public static BoolStorage fillMissingBoolStorage(BoolStorage storage, boolean fi
newValues.andNot(isNothingMap);
}
var validity = ImmutableBitSet.allTrue(size);
return new BoolStorage(newValues, validity, size, storage.isNegated());
return new BoolStorage(
new ImmutableBitSet(newValues, size), validity, size, storage.isNegated(), null);
}

public BooleanFillMissingOperation(StorageType<?> resultType) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ protected ColumnBooleanStorage applySpecializedMapOverBoolStorage(
}

int size = (int) left.getSize();
BitSet values = left.getValues();
BitSet values = left.getValues().cloneBitSet();
if (left.isNegated()) {
var newMissing = new BitSet(size);
newMissing.flip(0, size);
Expand Down Expand Up @@ -110,10 +110,10 @@ protected ColumnBooleanStorage applySpecializedZipOverBoolStorage(
BoolStorage left, BoolStorage right, MapOperationProblemAggregator problemAggregator) {
int size = (int) left.getSize();
int rightSize = (int) right.getSize();
BitSet values = left.getValues();
BitSet values = left.getValues().cloneBitSet();

// Compute the output set
BitSet out = right.getValues().get(0, size);
BitSet out = right.getValues().cloneBitSet().get(0, size);
boolean negated;
if (left.isNegated()) {
if (right.isNegated()) {
Expand Down Expand Up @@ -209,7 +209,7 @@ protected ColumnBooleanStorage applySpecializedMapOverBoolStorage(
}

int size = (int) left.getSize();
BitSet values = left.getValues();
BitSet values = left.getValues().cloneBitSet();
if (left.isNegated()) {
var newValidity = left.getValidityMap().cloneBitSet();
newValidity.andNot(values);
Expand All @@ -232,10 +232,10 @@ protected ColumnBooleanStorage applySpecializedZipOverBoolStorage(
BoolStorage left, BoolStorage right, MapOperationProblemAggregator problemAggregator) {
int size = (int) left.getSize();
int rightSize = (int) right.getSize();
BitSet values = left.getValues();
BitSet values = left.getValues().cloneBitSet();

// Compute the output set
BitSet out = right.getValues().get(0, size);
BitSet out = right.getValues().cloneBitSet().get(0, size);
boolean negated;
if (left.isNegated()) {
if (right.isNegated()) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
package org.enso.table.data.column.operation.comparators;

import java.util.BitSet;
import org.enso.table.data.column.builder.Builder;
import org.enso.table.data.column.operation.BinaryOperationBoolean;
import org.enso.table.data.column.operation.BinaryOperationTyped;
import org.enso.table.data.column.operation.unary.NotOperation;
import org.enso.table.data.column.storage.BoolStorage;
import org.enso.table.data.column.storage.ColumnBooleanStorage;
import org.enso.table.data.table.problems.MapOperationProblemAggregator;
import org.enso.table.util.ImmutableBitSet;

final class BooleanComparators {
public static final BinaryOperationTyped<Boolean> EQ =
Expand Down Expand Up @@ -60,10 +60,15 @@ protected ColumnBooleanStorage applySpecializedMapOverBoolStorage(
boolean rightBoolean,
boolean rightIsNothing,
MapOperationProblemAggregator problemAggregator) {
var size = (int) left.getSize();
return rightBoolean
? NotOperation.applySpecializedBoolStorage(left)
: new BoolStorage(
new BitSet(), left.getValidityMap(), Builder.checkSize(left.getSize()), false);
ImmutableBitSet.allFalse(size),
left.getValidityMap(),
Builder.checkSize(size),
false,
null);
}
};

Expand All @@ -81,9 +86,14 @@ protected ColumnBooleanStorage applySpecializedMapOverBoolStorage(
boolean rightBoolean,
boolean rightIsNothing,
MapOperationProblemAggregator problemAggregator) {
var size = (int) left.getSize();
return rightBoolean
? new BoolStorage(
new BitSet(), left.getValidityMap(), Builder.checkSize(left.getSize()), true)
ImmutableBitSet.allFalse(size),
left.getValidityMap(),
Builder.checkSize(size),
true,
null)
: NotOperation.applySpecializedBoolStorage(left);
}
};
Expand All @@ -102,9 +112,10 @@ protected ColumnBooleanStorage applySpecializedMapOverBoolStorage(
boolean rightBoolean,
boolean rightIsNothing,
MapOperationProblemAggregator problemAggregator) {
var size = Builder.checkSize(left.getSize());
return rightBoolean
? new BoolStorage(
new BitSet(), left.getValidityMap(), Builder.checkSize(left.getSize()), false)
ImmutableBitSet.allFalse(size), left.getValidityMap(), size, false, null)
: left;
}
};
Expand All @@ -123,10 +134,15 @@ protected ColumnBooleanStorage applySpecializedMapOverBoolStorage(
boolean rightBoolean,
boolean rightIsNothing,
MapOperationProblemAggregator problemAggregator) {
var size = (int) left.getSize();
return rightBoolean
? left
: new BoolStorage(
new BitSet(), left.getValidityMap(), Builder.checkSize(left.getSize()), true);
ImmutableBitSet.allFalse(size),
left.getValidityMap(),
Builder.checkSize(left.getSize()),
true,
null);
}
};
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package org.enso.table.data.column.operation.unary;

import java.util.BitSet;
import java.util.function.DoublePredicate;
import org.enso.table.data.column.builder.Builder;
import org.enso.table.data.column.operation.StorageIterators;
Expand All @@ -14,6 +13,7 @@
import org.enso.table.data.column.storage.type.IntegerType;
import org.enso.table.data.column.storage.type.StorageType;
import org.enso.table.data.table.problems.MapOperationProblemAggregator;
import org.enso.table.util.ImmutableBitSet;

public class DoubleIsOperation implements UnaryOperation {
public static final String FINITE_NAME = "is_finite";
Expand Down Expand Up @@ -62,8 +62,13 @@ public ColumnStorage<?> apply(
// For Finite
if (isAllFinite(storage.getType())) {
if (storage instanceof ColumnStorageWithValidityMap withNothingMap) {
var size = (int) storage.getSize();
return new BoolStorage(
new BitSet(), withNothingMap.getValidityMap(), (int) storage.getSize(), finiteValue);
ImmutableBitSet.allFalse(size),
withNothingMap.getValidityMap(),
size,
finiteValue,
null);
}

return StorageIterators.mapOverStorage(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public ColumnStorage<?> apply(
if (storage instanceof ColumnStorageWithValidityMap validityMap) {
var size = (int) storage.getSize();
var allValidity = ImmutableBitSet.allTrue(size);
return new BoolStorage(validityMap.getValidityMap().cloneBitSet(), allValidity, size, true);
return new BoolStorage(validityMap.getValidityMap(), allValidity, size, true, null);
}

return StorageIterators.buildOverStorage(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,8 @@ public static ColumnBooleanStorage applySpecializedBoolStorage(BoolStorage boolS
boolStorage.getValues(),
boolStorage.getValidityMap(),
(int) boolStorage.getSize(),
!boolStorage.isNegated());
!boolStorage.isNegated(),
null);
}

public static ColumnBooleanStorage applySpecializedNullStorage(ColumnStorage<?> storage) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.enso.table.data.column.storage;

import java.lang.foreign.MemorySegment;
import java.util.BitSet;
import java.util.NoSuchElementException;
import org.enso.table.data.column.storage.iterators.ColumnBooleanStorageIterator;
Expand All @@ -9,21 +10,43 @@
/** A boolean column storage. */
public final class BoolStorage extends Storage<Boolean>
implements ColumnBooleanStorage, ColumnStorageWithValidityMap {
private final BitSet values;
private final ImmutableBitSet values;
private final ImmutableBitSet validityMap;
private final int size;
private final boolean negated;
private final ColumnStorage<?> proxy;

public BoolStorage(BitSet values, BitSet validityMap, int size, boolean negated) {
this(values, new ImmutableBitSet(validityMap, size), size, negated);
this(
new ImmutableBitSet(values, size),
new ImmutableBitSet(validityMap, size),
size,
negated,
null);
}

public BoolStorage(BitSet values, ImmutableBitSet validityMap, int size, boolean negated) {
public BoolStorage(
ImmutableBitSet values,
ImmutableBitSet validityMap,
int size,
boolean negated,
ColumnStorage<?> other) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
ColumnStorage<?> other) {
ColumnStorage<?> proxy) {

to make it clear?

Copy link
Member Author

Choose a reason for hiding this comment

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

If follows this pattern:

obrazek

PS: I didn't notice this comment before integration.

super(BooleanType.INSTANCE);
this.values = values;
this.validityMap = validityMap;
this.size = size;
this.negated = negated;
this.proxy = other;
}

@Override
public long addressOfData() {
return MemorySegment.ofBuffer(values.rawData()).address();
}

@Override
public long addressOfValidity() {
return MemorySegment.ofBuffer(validityMap.rawData()).address();
}

@Override
Expand Down Expand Up @@ -57,7 +80,7 @@ public boolean isNegated() {
return negated;
}

public BitSet getValues() {
public ImmutableBitSet getValues() {
return values;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package org.enso.base.polyglot.tests;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotSame;

import org.enso.table.data.column.builder.Builder;
import org.enso.test.utils.ContextUtils;
import org.junit.BeforeClass;
import org.junit.ClassRule;
import org.junit.Test;

public class BoolStorageTest {
@ClassRule
public static final ContextUtils ctx = ContextUtils.newBuilder("enso").assertGC(false).build();

@BeforeClass
public static void importAll() {
ctx.eval("enso", "from Standard.Base import all");
}

@Test
public void makeLocalFromLongStorage() {
var b = Builder.getForBoolean(3);
b.append(false).appendNulls(1).append(true);
var storage = b.seal();
var localStorage = Builder.makeLocal(storage);
assertNotSame("local storage is a copy of storage", storage, localStorage);
assertEquals("They have the same size", storage.getSize(), localStorage.getSize());
assertEquals("They have the same type", storage.getType(), localStorage.getType());
for (var i = 0L; i < storage.getSize(); i++) {
var elem = storage.getItemBoxed(i);
var localElem = localStorage.getItemBoxed(i);
assertEquals("At " + i, elem, localElem);
}
}
}
Loading
Loading