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

Custom kernel for converting timestamps b/n UTC and non-UTC non-DST timezones #1553

Merged
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
616e368
Semi-working kernel for timestamp timezone conversion
NVnavkumar Nov 9, 2023
ac5aa18
Updated gtest with transition list
NVnavkumar Nov 9, 2023
8c53cce
Refactor tests to use transitions as fixture
NVnavkumar Nov 9, 2023
c87f7fc
Add more items in the column to test each transition
NVnavkumar Nov 9, 2023
558b882
Updated unit gtests for timezone kernel
NVnavkumar Nov 10, 2023
e33bb3a
Implementation of GpuTimeZoneDB with matching interface with CPU POC.
NVnavkumar Nov 14, 2023
ca8502a
Add minimal convert from UTC test
NVnavkumar Nov 14, 2023
3a22b6d
Fix wrong offset bug in creating transition DB and update tests to sy…
NVnavkumar Nov 15, 2023
10476bc
Cleanup and sync test with CPP version.
NVnavkumar Nov 15, 2023
2f0f32a
Merge branch 'branch-23.12' into gpu-timezone-non-repeating-transition
NVnavkumar Nov 15, 2023
3094fe7
Fix bug that happens when we pass a timestamp on the transition time …
NVnavkumar Nov 15, 2023
5b7f09e
Update timezone handling for convert to UTC and update tests
NVnavkumar Nov 15, 2023
d78159a
Internalize the daemon thread running to cache the timezone db
NVnavkumar Nov 15, 2023
8b016b7
Fix null pointer exception by creating the instance automatically
NVnavkumar Nov 16, 2023
058c5cd
Fix the visibility of these methods.
NVnavkumar Nov 16, 2023
9d71bd4
Add comment to note the type of the column vector stored in the datab…
NVnavkumar Nov 16, 2023
21f7364
Remove the TIMESTAMP_DAYS code here.
NVnavkumar Nov 16, 2023
2c13b6d
Update this. I think the subtracting one second now doesn't make sens…
NVnavkumar Nov 16, 2023
7afbf1c
Update tests to handle around the instant of transition.
NVnavkumar Nov 16, 2023
8db2924
Add comments and add auto const
NVnavkumar Nov 16, 2023
0da8e78
Run clang format and cleanup
NVnavkumar Nov 16, 2023
0aadc89
Address some feedback:
NVnavkumar Nov 18, 2023
7a54d34
Convert device method to functor to clean up calls to thrust::transform
NVnavkumar Nov 18, 2023
8c4ddc9
Port Spark zoneId normalization to GpuTimeZoneDB
NVnavkumar Nov 21, 2023
e68beae
These need to be able to run independently of whether the timezone DB…
NVnavkumar Nov 21, 2023
52ae251
Fix logic of isSupportedTimeZone. Also lazy load GPU timezone databas…
NVnavkumar Nov 21, 2023
255cb23
Prevent a double close of fixed transitions HCV
NVnavkumar Nov 21, 2023
5588d06
Merge branch 'branch-23.12' of github.com:NVIDIA/spark-rapids-jni int…
NVnavkumar Nov 21, 2023
a190226
Address some feedback with some cleanup
NVnavkumar Nov 21, 2023
1b76f84
Refactor into template function
NVnavkumar Nov 21, 2023
6b4a496
Address more feedback by adding some aliases and cleanup
NVnavkumar Nov 21, 2023
7355eb4
Header reordering by near-to-far
NVnavkumar Nov 21, 2023
44ce6ae
Address some Java feedback
NVnavkumar Nov 21, 2023
cf0449c
Fix comment
NVnavkumar Nov 21, 2023
965e92a
fix formatting of JNI CPP file
NVnavkumar Nov 21, 2023
c3bfa14
Addressing more feedback, refactor again.
NVnavkumar Nov 21, 2023
a165262
Pre-commit clang-format
NVnavkumar Nov 21, 2023
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
2 changes: 2 additions & 0 deletions src/main/cpp/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ add_library(
src/CastStringJni.cpp
src/DateTimeRebaseJni.cpp
src/DecimalUtilsJni.cpp
src/GpuTimeZoneDBJni.cpp
src/HashJni.cpp
src/HistogramJni.cpp
src/MapUtilsJni.cpp
Expand All @@ -172,6 +173,7 @@ add_library(
src/murmur_hash.cu
src/parse_uri.cu
src/row_conversion.cu
src/timezones.cu
src/utilities.cu
src/xxhash64.cu
src/zorder.cu
Expand Down
51 changes: 51 additions & 0 deletions src/main/cpp/src/GpuTimeZoneDBJni.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/* Copyright (c) 2023, 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.
*/

#include "cudf_jni_apis.hpp"
#include "timezones.hpp"

extern "C" {

JNIEXPORT jlong JNICALL Java_com_nvidia_spark_rapids_jni_GpuTimeZoneDB_convertTimestampColumnToUTC(
JNIEnv *env, jclass, jlong input_handle, jlong transitions_handle, jint tz_index) {
JNI_NULL_CHECK(env, input_handle, "column is null", 0);
JNI_NULL_CHECK(env, transitions_handle, "column is null", 0);
try {
cudf::jni::auto_set_device(env);
auto const input = reinterpret_cast<cudf::column_view const*>(input_handle);
auto const transitions = reinterpret_cast<cudf::table_view const*>(transitions_handle);
auto const index = static_cast<cudf::size_type>(tz_index);
return cudf::jni::ptr_as_jlong(
spark_rapids_jni::convert_timestamp_to_utc(*input, *transitions, index).release());
}
CATCH_STD(env, 0);
}

JNIEXPORT jlong JNICALL Java_com_nvidia_spark_rapids_jni_GpuTimeZoneDB_convertUTCTimestampColumnToTimeZone(
JNIEnv *env, jclass, jlong input_handle, jlong transitions_handle, jint tz_index) {
JNI_NULL_CHECK(env, input_handle, "column is null", 0);
JNI_NULL_CHECK(env, transitions_handle, "column is null", 0);
try {
cudf::jni::auto_set_device(env);
auto const input = reinterpret_cast<cudf::column_view const*>(input_handle);
auto const transitions = reinterpret_cast<cudf::table_view const*>(transitions_handle);
auto const index = static_cast<cudf::size_type>(tz_index);
return cudf::jni::ptr_as_jlong(
spark_rapids_jni::convert_utc_timestamp_to_timezone(*input, *transitions, index).release());
}
CATCH_STD(env, 0);
}

}
206 changes: 206 additions & 0 deletions src/main/cpp/src/timezones.cu
Original file line number Diff line number Diff line change
@@ -0,0 +1,206 @@
/*
* Copyright (c) 2023, 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.
*/

#include <cudf/column/column.hpp>
#include <cudf/column/column_device_view.cuh>
#include <cudf/column/column_factories.hpp>
#include <cudf/detail/null_mask.hpp>
#include <cudf/lists/list_device_view.cuh>
#include <cudf/lists/lists_column_device_view.cuh>
#include <cudf/table/table.hpp>
#include <cudf/types.hpp>
#include <rmm/cuda_stream_view.hpp>
#include <rmm/exec_policy.hpp>
#include <thrust/binary_search.h>

#include "timezones.hpp"
Copy link
Collaborator

Choose a reason for hiding this comment

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

The headers should be grouped by a "near to far" order: local headers first, then cudf_test, then cudf/, then thrust, then rmm, finally C++ built-in. This applies for all C++ files.


using column = cudf::column;
using column_device_view = cudf::column_device_view;
using lists_column_device_view = cudf::detail::lists_column_device_view;
using size_type = cudf::size_type;
using struct_view = cudf::struct_view;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: This can be phrased more simply:

using cudf::column;
using cudf::column_device_view;
// etc.


namespace {

/**
* @brief Convert the timestamp value to either UTC or a specified timezone
*
* This device function uses a binary search to find the instant of the transition
* to find the right offset to do the transition.
*
* To transition to UTC: do a binary search on the tzInstant child column and subtract
* the offset
*
* To transition from UTC: do a binary search on the utcInstant child column and add
* the offset
NVnavkumar marked this conversation as resolved.
Show resolved Hide resolved
*
* @tparam typestamp_type type of the input and output timestamp
* @param timestamp input timestamp
* @param transitions the list column of transitions to figure out the correct offset
* to adjust the timestamp. The type of the values in this column is
* LIST<STRUCT<utcInstant: int64, tzInstant: int64, utcOffset: int32>>
* @param tz_index the index of the specified zone id in the transitions table
* @param to_utc whether we are converting to UTC or converting to the timezone
*/
template <typename timestamp_type>
mythrocks marked this conversation as resolved.
Show resolved Hide resolved
__device__ timestamp_type convert_timestamp_timezone(timestamp_type const &timestamp,
lists_column_device_view const &transitions,
size_type tz_index, bool to_utc) {

using duration_type = typename timestamp_type::duration;
using cuda::std::chrono::duration_cast;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please do not type alias any name having chrono:: prefix. It is very error-prone.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this specifically error prone?


auto epoch_seconds =
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
auto epoch_seconds =
auto const epoch_seconds =

static_cast<int64_t>(duration_cast<cudf::duration_s>(timestamp.time_since_epoch()).count());

auto const tz_transitions = cudf::list_device_view{transitions, tz_index};

auto size = tz_transitions.size();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit:

Suggested change
auto size = tz_transitions.size();
auto const size = tz_transitions.size();

Copy link
Collaborator

@ttnghia ttnghia Nov 17, 2023

Choose a reason for hiding this comment

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

  1. Also remove the empty line before this line.
  2. Please rename size to something more meaningful. I don't know size here is what size?


cudf::device_span<int64_t const> transition_times(
&(transitions.child()
.child(to_utc ? 1 : 0)
.data<int64_t>()[tz_transitions.element_offset(0)]),
static_cast<size_t>(size));

auto idx = thrust::upper_bound(thrust::seq, transition_times.begin(), transition_times.end(),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit:

Suggested change
auto idx = thrust::upper_bound(thrust::seq, transition_times.begin(), transition_times.end(),
auto const idx = thrust::upper_bound(thrust::seq, transition_times.begin(), transition_times.end(),

epoch_seconds) -
transition_times.begin();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do not subtract iterator from binary search like this. Instead, use like this:

auto const it = thrust::upper_bound(thrust::seq, begin, end, val);
auto const idx = static_cast<cudf::size_type>(thrust::distance(begin, it));


auto const list_offset = tz_transitions.element_offset(size_type(idx - 1));
ttnghia marked this conversation as resolved.
Show resolved Hide resolved
auto const utc_offset = duration_cast<duration_type>(cudf::duration_s{
static_cast<int64_t>(transitions.child().child(2).element<int32_t>(list_offset))});
return to_utc ? timestamp - utc_offset : timestamp + utc_offset;
Copy link
Collaborator

Choose a reason for hiding this comment

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

If to_utc were a template parameter, this would read as:

if constexpr (to_utc) {
  return timestamp - utc_offset;
}
else {
  return timestamp + utc_offset;
}

Verbose, but one branch would get eliminated.

}

} // namespace

namespace spark_rapids_jni {

std::unique_ptr<column> convert_timestamp_to_utc(cudf::column_view const &input,
cudf::table_view const &transitions,
size_type tz_index, rmm::cuda_stream_view stream,
rmm::mr::device_memory_resource *mr) {

auto type = input.type().id();
auto num_rows = input.size();
Copy link
Collaborator

Choose a reason for hiding this comment

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

auto const.


// get the fixed transitions
auto const ft_cdv_ptr = column_device_view::create(transitions.column(0), stream);
lists_column_device_view fixed_transitions = cudf::detail::lists_column_device_view{*ft_cdv_ptr};
NVnavkumar marked this conversation as resolved.
Show resolved Hide resolved

auto results = cudf::make_timestamp_column(input.type(), input.size(),
cudf::detail::copy_bitmask(input, stream, mr),
input.null_count(), stream, mr);

switch (type) {
case cudf::type_id::TIMESTAMP_SECONDS:
thrust::transform(rmm::exec_policy(stream), thrust::make_counting_iterator(0),
thrust::make_counting_iterator(num_rows),
results->mutable_view().begin<cudf::timestamp_s>(),
[input_data = input.begin<cudf::timestamp_s>(), fixed_transitions,
tz_index] __device__(auto const i) {
auto const timestamp = input_data[i];
return convert_timestamp_timezone<cudf::timestamp_s>(
timestamp, fixed_transitions, tz_index, true);
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since this transform is boilerplate, have we considered moving this to a separate function template, parametrized on timestamp_type?

Copy link
Collaborator

@ttnghia ttnghia Nov 17, 2023

Choose a reason for hiding this comment

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

Recommend to follow what is suggested here: #1553 (comment) with some modification. In particular, this code block also need to be extracted into that (template) functor.

break;
case cudf::type_id::TIMESTAMP_MILLISECONDS:
thrust::transform(rmm::exec_policy(stream), thrust::make_counting_iterator(0),
thrust::make_counting_iterator(num_rows),
results->mutable_view().begin<cudf::timestamp_ms>(),
[input_data = input.begin<cudf::timestamp_ms>(), fixed_transitions,
tz_index] __device__(auto const i) {
auto const timestamp = input_data[i];
return convert_timestamp_timezone<cudf::timestamp_ms>(
timestamp, fixed_transitions, tz_index, true);
});
break;
case cudf::type_id::TIMESTAMP_MICROSECONDS:
thrust::transform(rmm::exec_policy(stream), thrust::make_counting_iterator(0),
thrust::make_counting_iterator(num_rows),
results->mutable_view().begin<cudf::timestamp_us>(),
[input_data = input.begin<cudf::timestamp_us>(), fixed_transitions,
tz_index] __device__(auto const i) {
auto const timestamp = input_data[i];
return convert_timestamp_timezone<cudf::timestamp_us>(
timestamp, fixed_transitions, tz_index, true);
});
break;
default: CUDF_FAIL("Unsupported timestamp unit for timezone conversion");
}

return results;
}

std::unique_ptr<column> convert_utc_timestamp_to_timezone(cudf::column_view const &input,
cudf::table_view const &transitions,
size_type tz_index,
rmm::cuda_stream_view stream,
rmm::mr::device_memory_resource *mr) {

auto const type = input.type().id();
auto num_rows = input.size();

// get the fixed transitions
auto const ft_cdv_ptr = column_device_view::create(transitions.column(0), stream);
lists_column_device_view fixed_transitions = cudf::detail::lists_column_device_view{*ft_cdv_ptr};

auto results = cudf::make_timestamp_column(input.type(), input.size(),
cudf::detail::copy_bitmask(input, stream, mr),
input.null_count(), stream, mr);

switch (type) {
case cudf::type_id::TIMESTAMP_SECONDS:
thrust::transform(rmm::exec_policy(stream), thrust::make_counting_iterator(0),
thrust::make_counting_iterator(num_rows),
results->mutable_view().begin<cudf::timestamp_s>(),
[input_data = input.begin<cudf::timestamp_s>(), fixed_transitions,
tz_index] __device__(auto const i) {
auto const timestamp = input_data[i];
return convert_timestamp_timezone<cudf::timestamp_s>(
timestamp, fixed_transitions, tz_index, false);
});
break;
case cudf::type_id::TIMESTAMP_MILLISECONDS:
thrust::transform(rmm::exec_policy(stream), thrust::make_counting_iterator(0),
thrust::make_counting_iterator(num_rows),
results->mutable_view().begin<cudf::timestamp_ms>(),
[input_data = input.begin<cudf::timestamp_ms>(), fixed_transitions,
tz_index] __device__(auto const i) {
auto const timestamp = input_data[i];
return convert_timestamp_timezone<cudf::timestamp_ms>(
timestamp, fixed_transitions, tz_index, false);
});
break;
case cudf::type_id::TIMESTAMP_MICROSECONDS:
thrust::transform(rmm::exec_policy(stream), thrust::make_counting_iterator(0),
thrust::make_counting_iterator(num_rows),
results->mutable_view().begin<cudf::timestamp_us>(),
[input_data = input.begin<cudf::timestamp_us>(), fixed_transitions,
tz_index] __device__(auto const i) {
auto const timestamp = input_data[i];
return convert_timestamp_timezone<cudf::timestamp_us>(
timestamp, fixed_transitions, tz_index, false);
});
break;
default: CUDF_FAIL("Unsupported timestamp unit for timezone conversion");
}
return results;
}

} // namespace spark_rapids_jni
64 changes: 64 additions & 0 deletions src/main/cpp/src/timezones.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/*
* Copyright (c) 2023, 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.
*/

#include <cstddef>

#include <cudf/column/column_view.hpp>
#include <cudf/table/table.hpp>
#include <cudf/utilities/default_stream.hpp>
#include <rmm/cuda_stream_view.hpp>

namespace spark_rapids_jni {

/**
* @brief Convert input column timestamps in current timezone to UTC
*
* The transition rules are in enclosed in a table, and the index corresponding to the
* current timezone is given.
*
* This method is the converse of convert_utc_timestamp_to_timezone
NVnavkumar marked this conversation as resolved.
Show resolved Hide resolved
*
* @param input the column of input timestamps in the current timezone
* @param transitions the table of transitions for all timezones
* @param tz_index the index of the row in `transitions` corresponding to the current timezone
* @param stream CUDA stream used for device memory operations and kernel launches.
* @param mr Device memory resource used to allocate the returned timestamp column's memory
*/
std::unique_ptr<cudf::column> convert_timestamp_to_utc(
cudf::column_view const &input, cudf::table_view const &transitions, cudf::size_type tz_index,
rmm::cuda_stream_view stream = cudf::get_default_stream(),
rmm::mr::device_memory_resource *mr = rmm::mr::get_current_device_resource());

/**
* @brief Convert input column timestamps in UTC to specified timezone
*
* The transition rules are in enclosed in a table, and the index corresponding to the
* specific timezone is given.
*
* This method is the converse of convert_timestamp_to_utc
NVnavkumar marked this conversation as resolved.
Show resolved Hide resolved
*
* @param input the column of input timestamps in UTC
* @param transitions the table of transitions for all timezones
* @param tz_index the index of the row in `transitions` corresponding to the specific timezone
* @param stream CUDA stream used for device memory operations and kernel launches.
* @param mr Device memory resource used to allocate the returned timestamp column's memory
*/
std::unique_ptr<cudf::column> convert_utc_timestamp_to_timezone(
cudf::column_view const &input, cudf::table_view const &transitions, cudf::size_type tz_index,
rmm::cuda_stream_view stream = cudf::get_default_stream(),
rmm::mr::device_memory_resource *mr = rmm::mr::get_current_device_resource());

} // namespace spark_rapids_jni
3 changes: 3 additions & 0 deletions src/main/cpp/tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,9 @@ ConfigureTest(HASH
ConfigureTest(BLOOM_FILTER
bloom_filter.cu)

ConfigureTest(TIMEZONES
timezones.cpp)

ConfigureTest(UTILITIES
utilities.cpp)

Expand Down
Loading