Remove deprecated GpuTimeZoneDB methods#4422
Conversation
Drop the deprecated timezone cache overloads and renamed transition accessor now that the codebase no longer references them. Update the remaining timezone DB test to use the supported cache API. Made-with: Cursor Signed-off-by: Chong Gao <res_life@163.com>
7a4a8db to
da37046
Compare
Greptile SummaryThis PR removes three deprecated public methods from Key changes:
Confidence Score: 5/5Safe to merge — purely removes deprecated stubs with no functional impact on remaining APIs. All three removed methods were already no-ops with respect to their extra parameter (maxYear was never used) or direct renames of the current API (getTransitions == getTimezoneInfo). No caller logic is affected in this repository and the companion spark-rapids PR (#14500) handles the consuming side. The single test change is correct. No P1/P0 findings. No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant Caller
participant GpuTimeZoneDB
Note over Caller,GpuTimeZoneDB: Removed (deprecated) API surface
Caller->>GpuTimeZoneDB: ~~cacheDatabaseAsync(int maxYear)~~
Caller->>GpuTimeZoneDB: ~~cacheDatabase(int maxYear)~~
Caller->>GpuTimeZoneDB: ~~getTransitions()~~
Note over Caller,GpuTimeZoneDB: Current (supported) API surface
Caller->>GpuTimeZoneDB: cacheDatabaseAsync()
GpuTimeZoneDB->>GpuTimeZoneDB: cacheDatabaseImpl() [in new daemon thread]
Caller->>GpuTimeZoneDB: cacheDatabase()
GpuTimeZoneDB->>GpuTimeZoneDB: cacheDatabaseImpl() [synchronous]
Caller->>GpuTimeZoneDB: getTimezoneInfo()
GpuTimeZoneDB-->>Caller: Table(fixedTransitions, dstRules)
Reviews (3): Last reviewed commit: "Remove deprecated GpuTimeZoneDB methods" | Re-trigger Greptile |
|
Should first merge NVIDIA/spark-rapids#14500, then merge this. |
|
build |
firestarman
left a comment
There was a problem hiding this comment.
One nit but OK to merge it as is for me.
| /** | ||
| * This is deprecated, will be removed. | ||
| */ | ||
| public static void cacheDatabaseAsync(int maxYear) { |
There was a problem hiding this comment.
NIT:
Since this is an API of a Java library, we may have comments that this is deprecated, and the maxYear is ignored, because it calls cacheDatabaseAsync internally than just removing it.
public static void cacheDatabaseAsync(int maxYear) {
cacheDatabaseAsync()
}
### Description Previously, we use hybrid mode: - before 2200 year, generate a transition cache for DST timezone, and run on GPU for timezone rebasing. This mehod wastes GPU memories. - after 2200 year, use CPU to do timezone rebasing. This is slow. Currently, we do not use hybrid mode any more, so remove the deprecated methods. - Replace deprecated `GpuTimeZoneDB.cacheDatabaseAsync(int)` and `cacheDatabase(int)` usage with the no-arg APIs. - Remove the now-unused `spark.rapids.timezone.transitionCache.maxYear` config and obsolete docs entry. - Update timezone perf and cast tests to call the replacement API. ## Related change * NVIDIA/spark-rapids-jni#4422 ### Checklists - [x] This PR has added documentation for new or modified features or behaviors. - [x] This PR has added new tests or modified existing tests to cover new code paths. (Updated existing timezone perf and cast tests to exercise the replacement API entry points.) - [ ] Performance testing has been performed and its results are added in the PR description. Or, an issue has been filed with a link in the PR description. Signed-off-by: Chong Gao <chongg@nvidia.com> Signed-off-by: Chong Gao <res_life@163.com> Co-authored-by: Chong Gao <res_life@163.com>
|
build |
Summary
GpuTimeZoneDB.cacheDatabaseAsync(int),cacheDatabase(int), andgetTransitions()methodsRelated change