Skip to content

Commit 2a4ee44

Browse files
committed
chore: Remove needles from_raw calls
1 parent 3ad22eb commit 2a4ee44

4 files changed

Lines changed: 117 additions & 152 deletions

File tree

native/core/src/errors.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -522,7 +522,7 @@ mod tests {
522522

523523
use jni::{
524524
objects::{JClass, JIntArray, JString, JThrowable},
525-
sys::{jintArray, jstring},
525+
sys::{jstring, jintArray},
526526
AttachGuard, InitArgsBuilder, JNIEnv, JNIVersion, JavaVM,
527527
};
528528

native/core/src/execution/jni_api.rs

Lines changed: 57 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -50,20 +50,16 @@ use datafusion_spark::function::string::char::CharFunc;
5050
use futures::poll;
5151
use futures::stream::StreamExt;
5252
use jni::objects::JByteBuffer;
53-
use jni::sys::JNI_FALSE;
53+
use jni::sys::{jlongArray, JNI_FALSE};
5454
use jni::{
5555
errors::Result as JNIResult,
5656
objects::{
57-
JByteArray, JClass, JIntArray, JLongArray, JObject, JObjectArray, JPrimitiveArray, JString,
58-
ReleaseMode,
57+
JByteArray, JClass, JIntArray, JLongArray, JObject, JObjectArray, JString,
58+
ReleaseMode,GlobalRef
5959
},
60-
sys::{jbyteArray, jint, jlong, jlongArray},
60+
sys::{jint, jlong, jboolean, jdouble},
6161
JNIEnv,
6262
};
63-
use jni::{
64-
objects::GlobalRef,
65-
sys::{jboolean, jdouble, jintArray, jobjectArray, jstring},
66-
};
6763
use std::collections::HashMap;
6864
use std::path::PathBuf;
6965
use std::time::{Duration, Instant};
@@ -159,26 +155,25 @@ pub unsafe extern "system" fn Java_org_apache_comet_Native_createPlan(
159155
e: JNIEnv,
160156
_class: JClass,
161157
id: jlong,
162-
iterators: jobjectArray,
163-
serialized_query: jbyteArray,
164-
serialized_spark_configs: jbyteArray,
158+
iterators: JObjectArray,
159+
serialized_query: JByteArray,
160+
serialized_spark_configs: JByteArray,
165161
partition_count: jint,
166162
metrics_node: JObject,
167163
metrics_update_interval: jlong,
168164
comet_task_memory_manager_obj: JObject,
169-
local_dirs: jobjectArray,
165+
local_dirs: JObjectArray,
170166
batch_size: jint,
171167
off_heap_mode: jboolean,
172-
memory_pool_type: jstring,
168+
memory_pool_type: JString,
173169
memory_limit: jlong,
174170
memory_limit_per_task: jlong,
175171
task_attempt_id: jlong,
176172
key_unwrapper_obj: JObject,
177173
) -> jlong {
178174
try_unwrap_or_throw(&e, |mut env| {
179175
// Deserialize Spark configs
180-
let array = unsafe { JPrimitiveArray::from_raw(serialized_spark_configs) };
181-
let bytes = env.convert_byte_array(array)?;
176+
let bytes = env.convert_byte_array(serialized_spark_configs)?;
182177
let spark_configs = serde::deserialize_config(bytes.as_slice())?;
183178
let spark_config: HashMap<String, String> = spark_configs.entries.into_iter().collect();
184179

@@ -196,18 +191,16 @@ pub unsafe extern "system" fn Java_org_apache_comet_Native_createPlan(
196191
let start = Instant::now();
197192

198193
// Deserialize query plan
199-
let array = unsafe { JPrimitiveArray::from_raw(serialized_query) };
200-
let bytes = env.convert_byte_array(array)?;
194+
let bytes = env.convert_byte_array(serialized_query)?;
201195
let spark_plan = serde::deserialize_op(bytes.as_slice())?;
202196

203197
let metrics = Arc::new(jni_new_global_ref!(env, metrics_node)?);
204198

205199
// Get the global references of input sources
206200
let mut input_sources = vec![];
207-
let iter_array = JObjectArray::from_raw(iterators);
208-
let num_inputs = env.get_array_length(&iter_array)?;
201+
let num_inputs = env.get_array_length(&iterators)?;
209202
for i in 0..num_inputs {
210-
let input_source = env.get_object_array_element(&iter_array, i)?;
203+
let input_source = env.get_object_array_element(&iterators, i)?;
211204
let input_source = Arc::new(jni_new_global_ref!(env, input_source)?);
212205
input_sources.push(input_source);
213206
}
@@ -216,7 +209,7 @@ pub unsafe extern "system" fn Java_org_apache_comet_Native_createPlan(
216209
let task_memory_manager =
217210
Arc::new(jni_new_global_ref!(env, comet_task_memory_manager_obj)?);
218211

219-
let memory_pool_type = env.get_string(&JString::from_raw(memory_pool_type))?.into();
212+
let memory_pool_type = env.get_string(&memory_pool_type)?.into();
220213
let memory_pool_config = parse_memory_pool_config(
221214
off_heap_mode != JNI_FALSE,
222215
memory_pool_type,
@@ -227,13 +220,12 @@ pub unsafe extern "system" fn Java_org_apache_comet_Native_createPlan(
227220
create_memory_pool(&memory_pool_config, task_memory_manager, task_attempt_id);
228221

229222
// Get local directories for storing spill files
230-
let local_dirs_array = JObjectArray::from_raw(local_dirs);
231-
let num_local_dirs = env.get_array_length(&local_dirs_array)?;
232-
let mut local_dirs = vec![];
223+
let num_local_dirs = env.get_array_length(&local_dirs)?;
224+
let mut local_dirs_vec = vec![];
233225
for i in 0..num_local_dirs {
234-
let local_dir: JString = env.get_object_array_element(&local_dirs_array, i)?.into();
226+
let local_dir: JString = env.get_object_array_element(&local_dirs, i)?.into();
235227
let local_dir = env.get_string(&local_dir)?;
236-
local_dirs.push(local_dir.into());
228+
local_dirs_vec.push(local_dir.into());
237229
}
238230

239231
// We need to keep the session context alive. Some session state like temporary
@@ -242,7 +234,7 @@ pub unsafe extern "system" fn Java_org_apache_comet_Native_createPlan(
242234
let session = prepare_datafusion_session_context(
243235
batch_size as usize,
244236
memory_pool,
245-
local_dirs,
237+
local_dirs_vec,
246238
max_temp_directory_size,
247239
)?;
248240

@@ -344,21 +336,19 @@ fn prepare_datafusion_session_context(
344336
/// Prepares arrow arrays for output.
345337
fn prepare_output(
346338
env: &mut JNIEnv,
347-
array_addrs: jlongArray,
348-
schema_addrs: jlongArray,
339+
array_addrs: JLongArray,
340+
schema_addrs: JLongArray,
349341
output_batch: RecordBatch,
350342
validate: bool,
351343
) -> CometResult<jlong> {
352-
let array_address_array = unsafe { JLongArray::from_raw(array_addrs) };
353-
let num_cols = env.get_array_length(&array_address_array)? as usize;
344+
let num_cols = env.get_array_length(&array_addrs)? as usize;
354345

355346
let array_addrs =
356-
unsafe { env.get_array_elements(&array_address_array, ReleaseMode::NoCopyBack)? };
347+
unsafe { env.get_array_elements(&array_addrs, ReleaseMode::NoCopyBack)? };
357348
let array_addrs = &*array_addrs;
358349

359-
let schema_address_array = unsafe { JLongArray::from_raw(schema_addrs) };
360350
let schema_addrs =
361-
unsafe { env.get_array_elements(&schema_address_array, ReleaseMode::NoCopyBack)? };
351+
unsafe { env.get_array_elements(&schema_addrs, ReleaseMode::NoCopyBack)? };
362352
let schema_addrs = &*schema_addrs;
363353

364354
let results = output_batch.columns();
@@ -441,8 +431,8 @@ pub unsafe extern "system" fn Java_org_apache_comet_Native_executePlan(
441431
stage_id: jint,
442432
partition: jint,
443433
exec_context: jlong,
444-
array_addrs: jlongArray,
445-
schema_addrs: jlongArray,
434+
array_addrs: JLongArray,
435+
schema_addrs: JLongArray,
446436
) -> jlong {
447437
try_unwrap_or_throw(&e, |mut env| {
448438
// Retrieve the query
@@ -599,24 +589,21 @@ fn update_metrics(env: &mut JNIEnv, exec_context: &mut ExecutionContext) -> Come
599589

600590
fn convert_datatype_arrays(
601591
env: &'_ mut JNIEnv<'_>,
602-
serialized_datatypes: jobjectArray,
592+
serialized_datatypes: JObjectArray,
603593
) -> JNIResult<Vec<ArrowDataType>> {
604-
unsafe {
605-
let obj_array = JObjectArray::from_raw(serialized_datatypes);
606-
let array_len = env.get_array_length(&obj_array)?;
607-
let mut res: Vec<ArrowDataType> = Vec::new();
608-
609-
for i in 0..array_len {
610-
let inner_array = env.get_object_array_element(&obj_array, i)?;
611-
let inner_array: JByteArray = inner_array.into();
612-
let bytes = env.convert_byte_array(inner_array)?;
613-
let data_type = serde::deserialize_data_type(bytes.as_slice()).unwrap();
614-
let arrow_dt = to_arrow_datatype(&data_type);
615-
res.push(arrow_dt);
616-
}
617-
618-
Ok(res)
594+
let array_len = env.get_array_length(&serialized_datatypes)?;
595+
let mut res: Vec<ArrowDataType> = Vec::new();
596+
597+
for i in 0..array_len {
598+
let inner_array = env.get_object_array_element(&serialized_datatypes, i)?;
599+
let inner_array: JByteArray = inner_array.into();
600+
let bytes = env.convert_byte_array(inner_array)?;
601+
let data_type = serde::deserialize_data_type(bytes.as_slice()).unwrap();
602+
let arrow_dt = to_arrow_datatype(&data_type);
603+
res.push(arrow_dt);
619604
}
605+
606+
Ok(res)
620607
}
621608

622609
fn get_execution_context<'a>(id: i64) -> &'a mut ExecutionContext {
@@ -634,16 +621,16 @@ fn get_execution_context<'a>(id: i64) -> &'a mut ExecutionContext {
634621
pub unsafe extern "system" fn Java_org_apache_comet_Native_writeSortedFileNative(
635622
e: JNIEnv,
636623
_class: JClass,
637-
row_addresses: jlongArray,
638-
row_sizes: jintArray,
639-
serialized_datatypes: jobjectArray,
640-
file_path: jstring,
624+
row_addresses: JLongArray,
625+
row_sizes: JIntArray,
626+
serialized_datatypes: JObjectArray,
627+
file_path: JString,
641628
prefer_dictionary_ratio: jdouble,
642629
batch_size: jlong,
643630
checksum_enabled: jboolean,
644631
checksum_algo: jint,
645632
current_checksum: jlong,
646-
compression_codec: jstring,
633+
compression_codec: JString,
647634
compression_level: jint,
648635
tracing_enabled: jboolean,
649636
) -> jlongArray {
@@ -654,19 +641,17 @@ pub unsafe extern "system" fn Java_org_apache_comet_Native_writeSortedFileNative
654641
|| {
655642
let data_types = convert_datatype_arrays(&mut env, serialized_datatypes)?;
656643

657-
let row_address_array = JLongArray::from_raw(row_addresses);
658-
let row_num = env.get_array_length(&row_address_array)? as usize;
644+
let row_num = env.get_array_length(&row_addresses)? as usize;
659645
let row_addresses =
660-
env.get_array_elements(&row_address_array, ReleaseMode::NoCopyBack)?;
646+
env.get_array_elements(&row_addresses, ReleaseMode::NoCopyBack)?;
661647

662-
let row_size_array = JIntArray::from_raw(row_sizes);
663-
let row_sizes = env.get_array_elements(&row_size_array, ReleaseMode::NoCopyBack)?;
648+
let row_sizes = env.get_array_elements(&row_sizes, ReleaseMode::NoCopyBack)?;
664649

665650
let row_addresses_ptr = row_addresses.as_ptr();
666651
let row_sizes_ptr = row_sizes.as_ptr();
667652

668653
let output_path: String = env
669-
.get_string(&JString::from_raw(file_path))
654+
.get_string(&file_path)
670655
.unwrap()
671656
.into();
672657

@@ -679,7 +664,7 @@ pub unsafe extern "system" fn Java_org_apache_comet_Native_writeSortedFileNative
679664
};
680665

681666
let compression_codec: String = env
682-
.get_string(&JString::from_raw(compression_codec))
667+
.get_string(&compression_codec)
683668
.unwrap()
684669
.into();
685670

@@ -754,8 +739,8 @@ pub unsafe extern "system" fn Java_org_apache_comet_Native_decodeShuffleBlock(
754739
_class: JClass,
755740
byte_buffer: JByteBuffer,
756741
length: jint,
757-
array_addrs: jlongArray,
758-
schema_addrs: jlongArray,
742+
array_addrs: JLongArray,
743+
schema_addrs: JLongArray,
759744
tracing_enabled: jboolean,
760745
) -> jlong {
761746
try_unwrap_or_throw(&e, |mut env| {
@@ -775,10 +760,10 @@ pub unsafe extern "system" fn Java_org_apache_comet_Native_decodeShuffleBlock(
775760
pub unsafe extern "system" fn Java_org_apache_comet_Native_traceBegin(
776761
e: JNIEnv,
777762
_class: JClass,
778-
event: jstring,
763+
event: JString,
779764
) {
780765
try_unwrap_or_throw(&e, |mut env| {
781-
let name: String = env.get_string(&JString::from_raw(event)).unwrap().into();
766+
let name: String = env.get_string(&event).unwrap().into();
782767
trace_begin(&name);
783768
Ok(())
784769
})
@@ -790,10 +775,10 @@ pub unsafe extern "system" fn Java_org_apache_comet_Native_traceBegin(
790775
pub unsafe extern "system" fn Java_org_apache_comet_Native_traceEnd(
791776
e: JNIEnv,
792777
_class: JClass,
793-
event: jstring,
778+
event: JString,
794779
) {
795780
try_unwrap_or_throw(&e, |mut env| {
796-
let name: String = env.get_string(&JString::from_raw(event)).unwrap().into();
781+
let name: String = env.get_string(&event).unwrap().into();
797782
trace_end(&name);
798783
Ok(())
799784
})
@@ -805,11 +790,11 @@ pub unsafe extern "system" fn Java_org_apache_comet_Native_traceEnd(
805790
pub unsafe extern "system" fn Java_org_apache_comet_Native_logMemoryUsage(
806791
e: JNIEnv,
807792
_class: JClass,
808-
name: jstring,
793+
name: JString,
809794
value: jlong,
810795
) {
811796
try_unwrap_or_throw(&e, |mut env| {
812-
let name: String = env.get_string(&JString::from_raw(name)).unwrap().into();
797+
let name: String = env.get_string(&name).unwrap().into();
813798
log_memory_usage(&name, value as u64);
814799
Ok(())
815800
})

0 commit comments

Comments
 (0)