Skip to content

Commit e69ad10

Browse files
committed
Delay path parsing until SourceFilePath creation.
1 parent 1c09c8d commit e69ad10

File tree

13 files changed

+302
-253
lines changed

13 files changed

+302
-253
lines changed

samply-api/src/api_file_path.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
1+
use std::borrow::Cow;
2+
13
use samply_symbols::SourceFilePath;
24

3-
pub fn to_api_file_path(file_path: &SourceFilePath) -> String {
4-
match file_path.mapped_path() {
5-
Some(mapped_path) => mapped_path.to_special_path_str(),
6-
None => file_path.raw_path().to_owned(),
7-
}
5+
pub fn to_api_file_path(file_path: &SourceFilePath) -> Cow<'_, str> {
6+
file_path
7+
.special_path_str()
8+
.unwrap_or_else(|| file_path.raw_path().into())
89
}

samply-symbols/src/breakpad/symbol_map.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -336,7 +336,7 @@ impl<T: FileContents> SymbolMapTrait for BreakpadSymbolMapInner<'_, T> {
336336
let file = files.get_string(inlinee.call_file).ok();
337337
frames.push(FrameDebugInfo {
338338
function: name,
339-
file_path: file.map(SourceFilePath::from_breakpad_path),
339+
file_path: file.map(SourceFilePath::BreakpadSpecialPathStr),
340340
line_number: Some(inlinee.call_line),
341341
});
342342
let inline_origin = inline_origins.get_string(inlinee.origin_id).ok();
@@ -352,7 +352,7 @@ impl<T: FileContents> SymbolMapTrait for BreakpadSymbolMapInner<'_, T> {
352352
};
353353
frames.push(FrameDebugInfo {
354354
function: name,
355-
file_path: file.map(SourceFilePath::from_breakpad_path),
355+
file_path: file.map(SourceFilePath::BreakpadSpecialPathStr),
356356
line_number,
357357
});
358358
frames.reverse();
@@ -449,31 +449,31 @@ mod test {
449449
frames[0],
450450
FrameDebugInfo {
451451
function: Some("WriteRelease64(long long*, long long)".into()),
452-
file_path: Some(SourceFilePath::new("/builds/worker/workspace/obj-build/browser/app/d:/agent/_work/2/s/src/externalapis/windows/10/sdk/inc/winnt.h".into(), None)),
452+
file_path: Some(SourceFilePath::BreakpadSpecialPathStr("/builds/worker/workspace/obj-build/browser/app/d:/agent/_work/2/s/src/externalapis/windows/10/sdk/inc/winnt.h".into())),
453453
line_number: Some(7729)
454454
}
455455
);
456456
assert_eq!(
457457
frames[1],
458458
FrameDebugInfo {
459459
function: Some("WritePointerRelease(void**, void*)".into()),
460-
file_path: Some(SourceFilePath::new("/builds/worker/workspace/obj-build/browser/app/d:/agent/_work/2/s/src/externalapis/windows/10/sdk/inc/winnt.h".into(), None)),
460+
file_path: Some(SourceFilePath::BreakpadSpecialPathStr("/builds/worker/workspace/obj-build/browser/app/d:/agent/_work/2/s/src/externalapis/windows/10/sdk/inc/winnt.h".into())),
461461
line_number: Some(8358)
462462
}
463463
);
464464
assert_eq!(
465465
frames[2],
466466
FrameDebugInfo {
467467
function: Some("DloadUnlock()".into()),
468-
file_path: Some(SourceFilePath::new("/builds/worker/workspace/obj-build/browser/app/d:/agent/_work/2/s/src/vctools/delayimp/dloadsup.h".into(), None)),
468+
file_path: Some(SourceFilePath::BreakpadSpecialPathStr("/builds/worker/workspace/obj-build/browser/app/d:/agent/_work/2/s/src/vctools/delayimp/dloadsup.h".into())),
469469
line_number: Some(345)
470470
}
471471
);
472472
assert_eq!(
473473
frames[3],
474474
FrameDebugInfo {
475475
function: Some("DloadAcquireSectionWriteAccess()".into()),
476-
file_path: Some(SourceFilePath::new("/builds/worker/workspace/obj-build/browser/app/d:/agent/_work/2/s/src/vctools/delayimp/dloadsup.h".into(), None)),
476+
file_path: Some(SourceFilePath::BreakpadSpecialPathStr("/builds/worker/workspace/obj-build/browser/app/d:/agent/_work/2/s/src/vctools/delayimp/dloadsup.h".into())),
477477
line_number: Some(665)
478478
}
479479
);

samply-symbols/src/dwarf.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,8 @@ pub fn convert_stack_frame<R: gimli::Reader>(
5151
None => None,
5252
};
5353
let file_path = frame.location.as_ref().and_then(|l| l.file).map(|file| {
54-
let mapped_path = path_mapper.map_path(file);
55-
SourceFilePath::new(file.into(), mapped_path)
54+
let _mapped_path = path_mapper.map_path(file); // TODO: remove path mapper everywhere
55+
SourceFilePath::RawPath(file.into())
5656
});
5757

5858
FrameDebugInfo {

samply-symbols/src/jitdump.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -284,7 +284,7 @@ impl<T: FileContents> JitDumpSymbolMapInner<'_, T> {
284284
let file_path = String::from_utf8_lossy(&entry.file_path.as_slice()).into_owned();
285285
let frame = FrameDebugInfo {
286286
function: Some(name.clone()),
287-
file_path: Some(SourceFilePath::new(file_path, None)),
287+
file_path: Some(SourceFilePath::RawPath(file_path)),
288288
line_number: Some(entry.line),
289289
};
290290
Some(FramesLookupResult::Available(vec![frame]))

samply-symbols/src/mapped_path.rs

Lines changed: 221 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,59 @@
1+
use std::borrow::Cow;
2+
3+
use nom::branch::alt;
4+
use nom::bytes::complete::{tag, take_till1, take_until1, take_while1};
5+
use nom::character::complete::one_of;
6+
use nom::error::ErrorKind;
7+
use nom::sequence::{delimited, preceded, terminated, tuple};
8+
use nom::Err;
9+
10+
#[derive(Debug, Clone, Hash, PartialEq, Eq)]
11+
pub enum UnparsedMappedPath {
12+
Url(String),
13+
BreakpadSpecialPath(String),
14+
RawPath(String),
15+
}
16+
17+
impl UnparsedMappedPath {
18+
pub fn from_special_path_str(breakpad_special_path: &str) -> Self {
19+
Self::BreakpadSpecialPath(breakpad_special_path.to_owned())
20+
}
21+
pub fn from_url(url: &str) -> Self {
22+
Self::Url(url.to_owned())
23+
}
24+
pub fn from_raw_path(raw_path: &str) -> Self {
25+
Self::RawPath(raw_path.to_owned())
26+
}
27+
pub fn parse(&self) -> Option<MappedPath> {
28+
match self {
29+
UnparsedMappedPath::Url(url) => MappedPath::from_url(url),
30+
UnparsedMappedPath::BreakpadSpecialPath(bsp) => MappedPath::from_special_path_str(bsp),
31+
UnparsedMappedPath::RawPath(raw) => MappedPath::from_raw_path(raw),
32+
}
33+
}
34+
pub fn display_path(&self) -> Option<Cow<'_, str>> {
35+
if let Some(mp) = self.parse() {
36+
let display_path = mp.display_path().into_owned();
37+
Some(display_path.into())
38+
} else {
39+
None
40+
}
41+
}
42+
pub fn special_path_str(&self) -> Option<Cow<'_, str>> {
43+
let mp = match self {
44+
UnparsedMappedPath::Url(url) => MappedPath::from_url(url),
45+
UnparsedMappedPath::BreakpadSpecialPath(bsp) => return Some(bsp.into()),
46+
UnparsedMappedPath::RawPath(raw) => MappedPath::from_raw_path(raw),
47+
};
48+
if let Some(mp) = mp {
49+
let display_path = mp.to_special_path_str();
50+
Some(display_path.into())
51+
} else {
52+
None
53+
}
54+
}
55+
}
56+
157
/// A special source file path for source files which are hosted online.
258
///
359
/// About "special path" strings: Special paths strings are a string serialization
@@ -63,6 +119,10 @@ impl MappedPath {
63119
parse_url(url)
64120
}
65121

122+
pub fn from_raw_path(raw_path: &str) -> Option<Self> {
123+
parse_raw_path(raw_path)
124+
}
125+
66126
/// Serialize this mapped path to a string, using the "special path" syntax.
67127
pub fn to_special_path_str(&self) -> String {
68128
match self {
@@ -83,17 +143,17 @@ impl MappedPath {
83143
}
84144

85145
/// Create a short, display-friendly form of this path.
86-
pub fn display_path(&self) -> String {
146+
pub fn display_path(&self) -> Cow<'_, str> {
87147
match self {
88-
MappedPath::Git { path, .. } => path.clone(),
89-
MappedPath::Hg { path, .. } => path.clone(),
90-
MappedPath::S3 { path, .. } => path.clone(),
148+
MappedPath::Git { path, .. } => path.into(),
149+
MappedPath::Hg { path, .. } => path.into(),
150+
MappedPath::S3 { path, .. } => path.into(),
91151
MappedPath::Cargo {
92152
crate_name,
93153
version,
94154
path,
95155
..
96-
} => format!("{crate_name}-{version}/{path}"),
156+
} => format!("{crate_name}-{version}/{path}").into(),
97157
}
98158
}
99159
}
@@ -202,12 +262,93 @@ fn parse_url(input: &str) -> Option<MappedPath> {
202262
digest,
203263
path,
204264
}
265+
} else if let Some(mapped_path) = parse_gitiles_url(input) {
266+
mapped_path
205267
} else {
206268
return None;
207269
};
208270
Some(mapped_path)
209271
}
210272

273+
fn parse_raw_path(raw_path: &str) -> Option<MappedPath> {
274+
if let Ok(p) = map_rustc_path(raw_path) {
275+
return Some(p);
276+
}
277+
if let Ok(p) = map_cargo_dep_path(raw_path) {
278+
return Some(p);
279+
}
280+
None
281+
}
282+
283+
fn map_rustc_path(input: &str) -> Result<MappedPath, nom::Err<nom::error::Error<&str>>> {
284+
// /rustc/c79419af0721c614d050f09b95f076da09d37b0d/library/std/src/rt.rs
285+
// /rustc/e1884a8e3c3e813aada8254edfa120e85bf5ffca\/library\std\src\rt.rs
286+
// /rustc/a178d0322ce20e33eac124758e837cbd80a6f633\library\std\src\rt.rs
287+
let (input, rev) = delimited(
288+
tag("/rustc/"),
289+
take_till1(|c| c == '/' || c == '\\'),
290+
take_while1(|c| c == '/' || c == '\\'),
291+
)(input)?;
292+
let path = input.replace('\\', "/");
293+
Ok(MappedPath::Git {
294+
repo: "github.com/rust-lang/rust".into(),
295+
path,
296+
rev: rev.to_owned(),
297+
})
298+
}
299+
300+
fn map_cargo_dep_path(input: &str) -> Result<MappedPath, nom::Err<nom::error::Error<&str>>> {
301+
// /Users/mstange/.cargo/registry/src/github.com-1ecc6299db9ec823/nom-7.1.3/src/bytes/complete.rs
302+
let (input, (registry, crate_name_and_version)) = preceded(
303+
tuple((
304+
alt((take_until1("/.cargo"), take_until1("\\.cargo"))),
305+
delimited(one_of("/\\"), tag(".cargo"), one_of("/\\")),
306+
terminated(tag("registry"), one_of("/\\")),
307+
terminated(tag("src"), one_of("/\\")),
308+
)),
309+
tuple((
310+
terminated(take_till1(|c| c == '/' || c == '\\'), one_of("/\\")),
311+
terminated(take_till1(|c| c == '/' || c == '\\'), one_of("/\\")),
312+
)),
313+
)(input)?;
314+
let (crate_name, version) = match crate_name_and_version.rfind('-') {
315+
Some(pos) => (
316+
&crate_name_and_version[..pos],
317+
&crate_name_and_version[(pos + 1)..],
318+
),
319+
None => {
320+
return Err(Err::Error(nom::error::Error::new(
321+
crate_name_and_version,
322+
ErrorKind::Digit,
323+
)))
324+
}
325+
};
326+
let path = input.replace('\\', "/");
327+
Ok(MappedPath::Cargo {
328+
registry: registry.to_owned(),
329+
crate_name: crate_name.to_owned(),
330+
version: version.to_owned(),
331+
path,
332+
})
333+
}
334+
335+
fn parse_gitiles_url(input: &str) -> Option<MappedPath> {
336+
// https://pdfium.googlesource.com/pdfium.git/+/dab1161c861cc239e48a17e1a5d729aa12785a53/core/fdrm/fx_crypt.cpp?format=TEXT
337+
// -> "git:pdfium.googlesource.com/pdfium:core/fdrm/fx_crypt.cpp:dab1161c861cc239e48a17e1a5d729aa12785a53"
338+
// https://chromium.googlesource.com/chromium/src.git/+/c15858db55ed54c230743eaa9678117f21d5517e/third_party/blink/renderer/core/svg/svg_point.cc?format=TEXT
339+
// -> "git:chromium.googlesource.com/chromium/src:third_party/blink/renderer/core/svg/svg_point.cc:c15858db55ed54c230743eaa9678117f21d5517e"
340+
let input = input
341+
.strip_prefix("https://")?
342+
.strip_suffix("?format=TEXT")?;
343+
let (repo, input) = input.split_once(".git/+/")?;
344+
let (rev, path) = input.split_once('/')?;
345+
Some(MappedPath::Git {
346+
repo: repo.to_owned(),
347+
path: path.to_owned(),
348+
rev: rev.to_owned(),
349+
})
350+
}
351+
211352
#[cfg(test)]
212353
mod test {
213354
use super::*;
@@ -372,4 +513,79 @@ mod test {
372513
"cargo:github.com-1ecc6299db9ec823:fxprof-processed-profile-0.3.0:src/lib.rs",
373514
);
374515
}
516+
517+
#[test]
518+
fn test_map_rustc_path() {
519+
assert_eq!(
520+
map_rustc_path(
521+
r#"/rustc/c79419af0721c614d050f09b95f076da09d37b0d/library/std/src/rt.rs"#
522+
),
523+
Ok(MappedPath::Git {
524+
repo: "github.com/rust-lang/rust".into(),
525+
path: "library/std/src/rt.rs".into(),
526+
rev: "c79419af0721c614d050f09b95f076da09d37b0d".into()
527+
})
528+
);
529+
assert_eq!(
530+
map_rustc_path(
531+
r"/rustc/e1884a8e3c3e813aada8254edfa120e85bf5ffca\/library\std\src\rt.rs"
532+
),
533+
Ok(MappedPath::Git {
534+
repo: "github.com/rust-lang/rust".into(),
535+
path: "library/std/src/rt.rs".into(),
536+
rev: "e1884a8e3c3e813aada8254edfa120e85bf5ffca".into()
537+
})
538+
);
539+
assert_eq!(
540+
map_rustc_path(
541+
r"/rustc/a178d0322ce20e33eac124758e837cbd80a6f633\library\std\src\rt.rs"
542+
),
543+
Ok(MappedPath::Git {
544+
repo: "github.com/rust-lang/rust".into(),
545+
path: "library/std/src/rt.rs".into(),
546+
rev: "a178d0322ce20e33eac124758e837cbd80a6f633".into()
547+
})
548+
);
549+
}
550+
551+
#[test]
552+
fn test_map_cargo_dep_path() {
553+
assert_eq!(
554+
map_cargo_dep_path(
555+
r#"/Users/mstange/.cargo/registry/src/github.com-1ecc6299db9ec823/nom-7.1.3/src/bytes/complete.rs"#
556+
),
557+
Ok(MappedPath::Cargo {
558+
registry: "github.com-1ecc6299db9ec823".into(),
559+
crate_name: "nom".into(),
560+
version: "7.1.3".into(),
561+
path: "src/bytes/complete.rs".into(),
562+
})
563+
);
564+
}
565+
566+
#[test]
567+
fn test_parse_gitiles_url() {
568+
assert_eq!(
569+
parse_gitiles_url("https://pdfium.googlesource.com/pdfium.git/+/dab1161c861cc239e48a17e1a5d729aa12785a53/core/fdrm/fx_crypt.cpp?format=TEXT"),
570+
Some(MappedPath::Git{
571+
repo: "pdfium.googlesource.com/pdfium".into(),
572+
rev: "dab1161c861cc239e48a17e1a5d729aa12785a53".into(),
573+
path: "core/fdrm/fx_crypt.cpp".into(),
574+
})
575+
);
576+
577+
assert_eq!(
578+
parse_gitiles_url("https://chromium.googlesource.com/chromium/src.git/+/c15858db55ed54c230743eaa9678117f21d5517e/third_party/blink/renderer/core/svg/svg_point.cc?format=TEXT"),
579+
Some(MappedPath::Git{
580+
repo: "chromium.googlesource.com/chromium/src".into(),
581+
rev: "c15858db55ed54c230743eaa9678117f21d5517e".into(),
582+
path: "third_party/blink/renderer/core/svg/svg_point.cc".into(),
583+
})
584+
);
585+
586+
assert_eq!(
587+
parse_gitiles_url("https://pdfium.googlesource.com/pdfium.git/+/dab1161c861cc239e48a17e1a5d729aa12785a53/core/fdrm/fx_crypt.cpp?format=TEXTotherstuff"),
588+
None
589+
);
590+
}
375591
}

0 commit comments

Comments
 (0)