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

Make ai.rapids.cudf.HostMemoryBuffer#copyFromStream public. #17179

Merged
merged 10 commits into from
Oct 30, 2024
78 changes: 78 additions & 0 deletions java/src/main/java/ai/rapids/cudf/Arms.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
/*
*
* Copyright (c) 2024, NVIDIA CORPORATION.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
*/

package ai.rapids.cudf;

import java.util.Arrays;
import java.util.Iterator;
import java.util.function.Function;

/**
* This class contains utility methods for automatic resource management.
*/
public class Arms {
/**
* This method close the resource if an exception is thrown while executing the function.
*/
public static <R extends AutoCloseable, T> T closeIfException(R resource, Function<R, T> function) {
try {
return function.apply(resource);
} catch (Exception e) {
if (resource != null) {
try {
resource.close();
} catch (Exception inner) {
e.addSuppressed(inner);
}
}
throw e;
}
}

/**
* This method safes closes the resources.
jlowe marked this conversation as resolved.
Show resolved Hide resolved
*/
public static <R extends AutoCloseable> void close(Iterator<R> resources) {
Throwable t = null;
while (resources.hasNext()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

First off this is not closing the iterator. This is closing all of the values that the iterator holds. So at a minimum I would like it if we changed the name to closeAll, or something like that.

Second an Iterator has no guarantee that you are looping over actual values. It can lazily generate values or things like that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed, I have changed method name to closeAll.

try {
resources.next().close();
} catch (Exception e) {
if (t == null) {
t = e;
} else {
t.addSuppressed(e);
}
}
jlowe marked this conversation as resolved.
Show resolved Hide resolved
}
}

/**
* This method safes closes the resources.
*/
public static <R extends AutoCloseable> void close(R... resources) {
close(Arrays.asList(resources));
}

/**
* This method safes closes the resources.
*/
public static <R extends AutoCloseable> void close(Iterable<R> resources) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This has the same problems as the Iterator API, but worse. An Iterable is typically a Collection or something like that. If that collection is AutoClosable then close is now ambiguous. Do we want to close the collection itself or do we want to close the things in that collection. This needs to be called closeAll at least. I would also prefer it if we switched this to Collection instead of Iterable, unless there is a specific use case where we want an Iterable that is not a Collection.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed, renamed to closeAll and use Collection.

close(resources.iterator());
}
}
2 changes: 1 addition & 1 deletion java/src/main/java/ai/rapids/cudf/HostMemoryBuffer.java
liurenjie1024 marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ public final void copyFromHostBuffer(long destOffset, HostMemoryBuffer srcData,
* @param in input stream to copy bytes from
* @param byteLength number of bytes to copy
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

Please update the docs so that it is clear that an EOFException is thrown if the InputStream does not include enough bytes to do the copy. Also it would really be nice if we could include a message with the EOFException to indicate what happened.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

final void copyFromStream(long destOffset, InputStream in, long byteLength) throws IOException {
public final void copyFromStream(long destOffset, InputStream in, long byteLength) throws IOException {
addressOutOfBoundsCheck(address + destOffset, byteLength, "copy from stream");
byte[] arrayBuffer = new byte[(int) Math.min(1024 * 128, byteLength)];
long left = byteLength;
Expand Down
44 changes: 44 additions & 0 deletions java/src/main/java/ai/rapids/cudf/Pair.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/*
*
* Copyright (c) 2024, NVIDIA CORPORATION.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
*/

package ai.rapids.cudf;

/**
* A utility class for holding a pair of values.
*/
public class Pair<K, V> {
private final K left;
private final V right;

public Pair(K left, V right) {
this.left = left;
this.right = right;
}

public K getLeft() {
return left;
}

public V getRight() {
return right;
}

public static <K, V> Pair<K, V> of(K left, V right) {
return new Pair<>(left, right);
}
}
43 changes: 43 additions & 0 deletions java/src/main/java/ai/rapids/cudf/Preconditions.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/*
*
* Copyright (c) 2024, NVIDIA CORPORATION.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
*/
package ai.rapids.cudf;

import java.util.function.Supplier;

/**
* This class contains utility methods for checking preconditions.
*/
public class Preconditions {
/**
* Check if the condition is true, otherwise throw an IllegalStateException with the given message.
*/
public static void ensure(boolean condition, String message) {
if (!condition) {
throw new IllegalStateException(message);
}
}

/**
* Check if the condition is true, otherwise throw an IllegalStateException with the given message supplier.
*/
public static void ensure(boolean condition, Supplier<String> messageSupplier) {
if (!condition) {
throw new IllegalStateException(messageSupplier.get());
}
}
}
65 changes: 65 additions & 0 deletions java/src/main/java/ai/rapids/cudf/SlicedTable.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
/*
*
* Copyright (c) 2024, NVIDIA CORPORATION.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
*/

package ai.rapids.cudf;

import java.util.Objects;

import static ai.rapids.cudf.Preconditions.ensure;

/**
* A sliced view to underlying table.
*
* This is a simple wrapper around a table that represents a slice of the table, and it doesn't change ownership of the
* underlying table, so it's always the caller's responsibility to manage the lifecycle of the underlying table.
*/
public class SlicedTable {
liurenjie1024 marked this conversation as resolved.
Show resolved Hide resolved
private final int startRow;
private final int numRows;
private final Table table;

public SlicedTable(int startRow, int numRows, Table table) {
Objects.requireNonNull(table, "table must not be null");
ensure(startRow >= 0, "startRow must be >= 0");
ensure(startRow < table.getRowCount(),
() -> "startRow " + startRow + " is larger than table row count " + table.getRowCount());
ensure(numRows >= 0, () -> "numRows " + numRows + " is negative");
ensure(startRow + numRows <= table.getRowCount(), () -> "startRow + numRows is " + (startRow + numRows)
+ ", must be less than table row count " + table.getRowCount());

this.startRow = startRow;
this.numRows = numRows;
this.table = table;
}

public int getStartRow() {
return startRow;
}

public int getNumRows() {
return numRows;
}

public Table getBaseTable() {
return table;
}

public static SlicedTable from(Table table, int startRow, int numRows) {
return new SlicedTable(startRow, numRows, table);
}
}
Loading