Skip to content

Commit 8945950

Browse files
committed
[profiling] Reduce copying and allocation in exporter
1 parent 9a4a791 commit 8945950

File tree

5 files changed

+92
-184
lines changed

5 files changed

+92
-184
lines changed

profiling-ffi/src/exporter.rs

+27-103
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,10 @@
66

77
use datadog_profiling::exporter;
88
use datadog_profiling::exporter::{ProfileExporter, Request};
9-
use datadog_profiling::internal::ProfiledEndpointsStats;
9+
use datadog_profiling::internal::EncodedProfile;
1010
use ddcommon::tag::Tag;
1111
use ddcommon_ffi::slice::{AsBytes, ByteSlice, CharSlice, Slice};
12-
use ddcommon_ffi::{Error, MaybeError, Timespec};
12+
use ddcommon_ffi::{Error, Handle, MaybeError, ToInner};
1313
use std::borrow::Cow;
1414
use std::ptr::NonNull;
1515
use std::str::FromStr;
@@ -259,16 +259,15 @@ impl From<RequestBuildResult> for Result<Box<Request>, String> {
259259
/// valid objects created by this module.
260260
/// NULL is allowed for `optional_additional_tags`, `optional_endpoints_stats`,
261261
/// `optional_internal_metadata_json` and `optional_info_json`.
262+
/// Consumes the `SerializedProfile`
262263
#[no_mangle]
263264
#[must_use]
264265
pub unsafe extern "C" fn ddog_prof_Exporter_Request_build(
265266
exporter: Option<&mut ProfileExporter>,
266-
start: Timespec,
267-
end: Timespec,
267+
mut profile: Handle<EncodedProfile>,
268268
files_to_compress_and_export: Slice<File>,
269269
files_to_export_unmodified: Slice<File>,
270270
optional_additional_tags: Option<&ddcommon_ffi::Vec<Tag>>,
271-
optional_endpoints_stats: Option<&ProfiledEndpointsStats>,
272271
optional_internal_metadata_json: Option<&CharSlice>,
273272
optional_info_json: Option<&CharSlice>,
274273
) -> RequestBuildResult {
@@ -290,13 +289,16 @@ pub unsafe extern "C" fn ddog_prof_Exporter_Request_build(
290289
Err(err) => return RequestBuildResult::Err(err.into()),
291290
};
292291

292+
let profile = match profile.take() {
293+
Ok(p) => *p,
294+
Err(e) => return RequestBuildResult::Err(e.into()),
295+
};
296+
293297
match exporter.build(
294-
start.into(),
295-
end.into(),
298+
profile,
296299
files_to_compress_and_export.as_slice(),
297300
files_to_export_unmodified.as_slice(),
298301
tags.as_ref(),
299-
optional_endpoints_stats,
300302
internal_metadata,
301303
info,
302304
) {
@@ -575,19 +577,7 @@ mod tests {
575577
ExporterNewResult::Err(_) => panic!("Should not occur!"),
576578
};
577579

578-
let files_to_compress_and_export: &[File] = &[File {
579-
name: CharSlice::from("foo.pprof"),
580-
file: ByteSlice::from(b"dummy contents" as &[u8]),
581-
}];
582-
583-
let start = Timespec {
584-
seconds: 12,
585-
nanoseconds: 34,
586-
};
587-
let finish = Timespec {
588-
seconds: 56,
589-
nanoseconds: 78,
590-
};
580+
let profile = EncodedProfile::test_instance().unwrap().into();
591581
let timeout_milliseconds = 90;
592582
unsafe {
593583
ddog_prof_Exporter_set_timeout(Some(exporter.as_mut()), timeout_milliseconds)
@@ -597,11 +587,9 @@ mod tests {
597587
let build_result = unsafe {
598588
ddog_prof_Exporter_Request_build(
599589
Some(exporter.as_mut()),
600-
start,
601-
finish,
602-
Slice::from(files_to_compress_and_export),
590+
profile,
591+
Slice::empty(),
603592
Slice::empty(),
604-
None,
605593
None,
606594
None,
607595
None,
@@ -649,19 +637,7 @@ mod tests {
649637
ExporterNewResult::Err(_) => panic!("Should not occur!"),
650638
};
651639

652-
let files: &[File] = &[File {
653-
name: CharSlice::from("foo.pprof"),
654-
file: ByteSlice::from(b"dummy contents" as &[u8]),
655-
}];
656-
657-
let start = Timespec {
658-
seconds: 12,
659-
nanoseconds: 34,
660-
};
661-
let finish = Timespec {
662-
seconds: 56,
663-
nanoseconds: 78,
664-
};
640+
let profile = EncodedProfile::test_instance().unwrap().into();
665641
let timeout_milliseconds = 90;
666642
unsafe {
667643
ddog_prof_Exporter_set_timeout(Some(exporter.as_mut()), timeout_milliseconds)
@@ -681,14 +657,12 @@ mod tests {
681657
let build_result = unsafe {
682658
ddog_prof_Exporter_Request_build(
683659
Some(exporter.as_mut()),
684-
start,
685-
finish,
686-
Slice::from(files),
660+
profile,
661+
Slice::empty(),
687662
Slice::empty(),
688663
None,
689664
None,
690665
Some(&raw_internal_metadata),
691-
None,
692666
)
693667
};
694668

@@ -724,19 +698,8 @@ mod tests {
724698
ExporterNewResult::Err(_) => panic!("Should not occur!"),
725699
};
726700

727-
let files: &[File] = &[File {
728-
name: CharSlice::from("foo.pprof"),
729-
file: ByteSlice::from(b"dummy contents" as &[u8]),
730-
}];
701+
let profile = EncodedProfile::test_instance().unwrap().into();
731702

732-
let start = Timespec {
733-
seconds: 12,
734-
nanoseconds: 34,
735-
};
736-
let finish = Timespec {
737-
seconds: 56,
738-
nanoseconds: 78,
739-
};
740703
let timeout_milliseconds = 90;
741704
unsafe {
742705
ddog_prof_Exporter_set_timeout(Some(exporter.as_mut()), timeout_milliseconds)
@@ -748,11 +711,9 @@ mod tests {
748711
let build_result = unsafe {
749712
ddog_prof_Exporter_Request_build(
750713
Some(exporter.as_mut()),
751-
start,
752-
finish,
753-
Slice::from(files),
714+
profile,
715+
Slice::empty(),
754716
Slice::empty(),
755-
None,
756717
None,
757718
Some(&raw_internal_metadata),
758719
None,
@@ -787,19 +748,7 @@ mod tests {
787748
ExporterNewResult::Err(_) => panic!("Should not occur!"),
788749
};
789750

790-
let files: &[File] = &[File {
791-
name: CharSlice::from("foo.pprof"),
792-
file: ByteSlice::from(b"dummy contents" as &[u8]),
793-
}];
794-
795-
let start = Timespec {
796-
seconds: 12,
797-
nanoseconds: 34,
798-
};
799-
let finish = Timespec {
800-
seconds: 56,
801-
nanoseconds: 78,
802-
};
751+
let profile = EncodedProfile::test_instance().unwrap().into();
803752
let timeout_milliseconds = 90;
804753
unsafe {
805754
ddog_prof_Exporter_set_timeout(Some(exporter.as_mut()), timeout_milliseconds)
@@ -840,11 +789,9 @@ mod tests {
840789
let build_result = unsafe {
841790
ddog_prof_Exporter_Request_build(
842791
Some(exporter.as_mut()),
843-
start,
844-
finish,
845-
Slice::from(files),
792+
profile,
793+
Slice::empty(),
846794
Slice::empty(),
847-
None,
848795
None,
849796
None,
850797
Some(&raw_info),
@@ -904,19 +851,7 @@ mod tests {
904851
ExporterNewResult::Err(_) => panic!("Should not occur!"),
905852
};
906853

907-
let files: &[File] = &[File {
908-
name: CharSlice::from("foo.pprof"),
909-
file: ByteSlice::from(b"dummy contents" as &[u8]),
910-
}];
911-
912-
let start = Timespec {
913-
seconds: 12,
914-
nanoseconds: 34,
915-
};
916-
let finish = Timespec {
917-
seconds: 56,
918-
nanoseconds: 78,
919-
};
854+
let profile = EncodedProfile::test_instance().unwrap().into();
920855
let timeout_milliseconds = 90;
921856
unsafe {
922857
ddog_prof_Exporter_set_timeout(Some(exporter.as_mut()), timeout_milliseconds)
@@ -928,11 +863,9 @@ mod tests {
928863
let build_result = unsafe {
929864
ddog_prof_Exporter_Request_build(
930865
Some(exporter.as_mut()),
931-
start,
932-
finish,
933-
Slice::from(files),
866+
profile,
867+
Slice::empty(),
934868
Slice::empty(),
935-
None,
936869
None,
937870
None,
938871
Some(&raw_info),
@@ -949,26 +882,17 @@ mod tests {
949882

950883
#[test]
951884
fn test_build_failure() {
952-
let start = Timespec {
953-
seconds: 12,
954-
nanoseconds: 34,
955-
};
956-
let finish = Timespec {
957-
seconds: 56,
958-
nanoseconds: 78,
959-
};
885+
let profile = EncodedProfile::test_instance().unwrap().into();
960886

961887
let build_result = unsafe {
962888
ddog_prof_Exporter_Request_build(
963889
None, // No exporter, will fail
964-
start,
965-
finish,
890+
profile,
966891
Slice::empty(),
967892
Slice::empty(),
968893
None,
969894
None,
970895
None,
971-
None,
972896
)
973897
};
974898

profiling-ffi/src/profiles.rs

+9-42
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,8 @@ use anyhow::Context;
77
use datadog_profiling::api;
88
use datadog_profiling::api::ManagedStringId;
99
use datadog_profiling::internal;
10-
use datadog_profiling::internal::ProfiledEndpointsStats;
1110
use ddcommon_ffi::slice::{AsBytes, CharSlice, Slice};
12-
use ddcommon_ffi::{Error, Timespec};
11+
use ddcommon_ffi::{Error, Handle, Timespec, ToInner};
1312
use std::num::NonZeroI64;
1413
use std::str::Utf8Error;
1514
use std::time::{Duration, SystemTime};
@@ -83,19 +82,10 @@ pub enum ProfileNewResult {
8382
#[allow(dead_code)]
8483
#[repr(C)]
8584
pub enum SerializeResult {
86-
Ok(EncodedProfile),
85+
Ok(Handle<internal::EncodedProfile>),
8786
Err(Error),
8887
}
8988

90-
impl From<anyhow::Result<EncodedProfile>> for SerializeResult {
91-
fn from(value: anyhow::Result<EncodedProfile>) -> Self {
92-
match value {
93-
Ok(e) => Self::Ok(e),
94-
Err(err) => Self::Err(err.into()),
95-
}
96-
}
97-
}
98-
9989
impl From<anyhow::Result<internal::EncodedProfile>> for SerializeResult {
10090
fn from(value: anyhow::Result<internal::EncodedProfile>) -> Self {
10191
match value {
@@ -719,41 +709,18 @@ unsafe fn add_upscaling_rule(
719709
)
720710
}
721711

722-
#[repr(C)]
723-
pub struct EncodedProfile {
724-
start: Timespec,
725-
end: Timespec,
726-
buffer: ddcommon_ffi::Vec<u8>,
727-
endpoints_stats: Box<ProfiledEndpointsStats>,
728-
}
729-
730712
/// # Safety
731713
/// Only pass a reference to a valid `ddog_prof_EncodedProfile`, or null. A
732714
/// valid reference also means that it hasn't already been dropped (do not
733715
/// call this twice on the same object).
734716
#[no_mangle]
735-
pub unsafe extern "C" fn ddog_prof_EncodedProfile_drop(profile: Option<&mut EncodedProfile>) {
736-
if let Some(reference) = profile {
737-
// Safety: EncodedProfile's are repr(C), and not box allocated. If the
738-
// user has followed the safety requirements of this function, then
739-
// this is safe.
740-
std::ptr::drop_in_place(reference as *mut _)
741-
}
742-
}
743-
744-
impl From<internal::EncodedProfile> for EncodedProfile {
745-
fn from(value: internal::EncodedProfile) -> Self {
746-
let start = value.start.into();
747-
let end = value.end.into();
748-
let buffer = value.buffer.into();
749-
let endpoints_stats = Box::new(value.endpoints_stats);
750-
751-
Self {
752-
start,
753-
end,
754-
buffer,
755-
endpoints_stats,
756-
}
717+
pub unsafe extern "C" fn ddog_prof_EncodedProfile_drop(
718+
profile: *mut Handle<internal::EncodedProfile>,
719+
) {
720+
// Technically, this function has been designed so if it's double-dropped
721+
// then it's okay, but it's not something that should be relied on.
722+
if !profile.is_null() {
723+
drop((*profile).take())
757724
}
758725
}
759726

0 commit comments

Comments
 (0)