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: github-unsubscr...@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org

Reply via email to