andygrove commented on code in PR #1029:
URL: https://github.com/apache/datafusion-comet/pull/1029#discussion_r1811520641
##########
native/core/src/execution/metrics/utils.rs:
##########
@@ -62,11 +67,30 @@ fn update_metrics(
env: &mut JNIEnv,
metric_node: &JObject,
metric_values: &[(&str, i64)],
+ metrics_jstrings: &mut HashMap<String, Arc<GlobalRef>>,
) -> Result<(), CometError> {
unsafe {
for &(name, value) in metric_values {
- let jname = jni_new_string!(env, &name)?;
- jni_call!(env, comet_metric_node(metric_node).set(&jname, value)
-> ())?;
+ // Perform a lookup in the jstrings cache.
+ if let Some(map_global_ref) = metrics_jstrings.get(name) {
+ // Cache hit. Extract the jstring from the global ref.
+ let jobject = map_global_ref.as_obj();
+ let jstring = JString::from_raw(**jobject);
+ // Update the metrics using the jstring as a key.
+ jni_call!(env, comet_metric_node(metric_node).set(&jstring,
value) -> ())?;
+ } else {
+ // Cache miss. Allocate a new string, promote to global ref,
and insert into cache.
+ let local_jstring = jni_new_string!(env, &name)?;
+ let global_ref = jni_new_global_ref!(env, local_jstring)?;
+ metrics_jstrings.insert(name.to_string(),
Arc::new(global_ref));
+ // try_insert returns a reference to the inserted value to
avoid the subsequent
+ // get on the kv pair that we just inserted, but it's still
experimental.
+ let map_global_ref = metrics_jstrings.get(name).unwrap();
Review Comment:
You can avoid the HashMap `get` call here by cloning the `Arc` before
inserting?
```suggestion
let map_global_ref = Arc::new(global_ref);
metrics_jstrings.insert(name.to_string(),
map_global_ref.clone());
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
