Skip to content

Commit f8f7c83

Browse files
muirdmmeta-codesync[bot]
muirdm
authored and
meta-codesync[bot]
committed
types: avoid allocating relativized paths
Summary: Add a non-allocating `RepoPathRelativizer::relativized()` display/serialize adapter and switch the current direct users in `cmdgrep` and `cmdstatus` to it. This removes repeated `String` allocation in hot output paths. In one warm `sl grep` case with many results, this reduced runtime from 13s to 9s. Reviewed By: SBones Differential Revision: D102545309 fbshipit-source-id: fc470692d98196f814a3f1a1e8b0eae008afc49e
1 parent 588560a commit f8f7c83

5 files changed

Lines changed: 223 additions & 53 deletions

File tree

‎eden/scm/lib/commands/commands/cmdgrep/src/biggrep.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -270,7 +270,7 @@ pub fn try_biggrep(
270270
lineno,
271271
context,
272272
} => {
273-
let rel_path = relativizer.relativize(&repo_path);
273+
let rel_path = relativizer.relativized(repo_path.as_repo_path());
274274
if files_with_matches {
275275
json_out.write(&GrepFileMatch { path: &rel_path })?;
276276
} else {
@@ -324,7 +324,7 @@ pub fn try_biggrep(
324324
lineno,
325325
context,
326326
} => {
327-
let rel_path = relativizer.relativize(&repo_path);
327+
let rel_path = relativizer.relativized(repo_path.as_repo_path());
328328
if files_with_matches {
329329
out.write_file_match(&rel_path)?;
330330
} else {
@@ -342,7 +342,7 @@ pub fn try_biggrep(
342342
match_count += 1;
343343
}
344344
BigGrepLine::BinaryMatch { repo_path } => {
345-
let rel_path = relativizer.relativize(&repo_path);
345+
let rel_path = relativizer.relativized(repo_path.as_repo_path());
346346
out.write_binary_match(&rel_path)?;
347347
match_count += 1;
348348
}

‎eden/scm/lib/commands/commands/cmdgrep/src/lib.rs‎

Lines changed: 32 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ use serde::Serialize;
4848
use termstyle::Styler;
4949
use types::RepoPathBuf;
5050
use types::hgid::WDIR_ID;
51+
use types::path::RelativizedRepoPath;
5152
use types::path::RepoPathRelativizer;
5253

5354
const OUTPUT_BUFFER_CAPACITY: usize = 64 * 1024;
@@ -142,8 +143,10 @@ impl<W: Write> PlainTextWriter<W> {
142143
inner.write_all(&style.suffix)
143144
}
144145

145-
fn write_path(&mut self, path: &str) -> io::Result<()> {
146-
Self::write_styled_bytes(&mut self.inner, &self.path_style, path.as_bytes())
146+
fn write_path(&mut self, path: impl std::fmt::Display) -> io::Result<()> {
147+
self.inner.write_all(&self.path_style.prefix)?;
148+
write!(self.inner, "{path}")?;
149+
self.inner.write_all(&self.path_style.suffix)
147150
}
148151

149152
fn write_line_number(&mut self, line_number: u64) -> io::Result<()> {
@@ -155,7 +158,12 @@ impl<W: Write> PlainTextWriter<W> {
155158
)
156159
}
157160

158-
fn write_prefix(&mut self, path: &str, line_number: Option<u64>, sep: u8) -> io::Result<()> {
161+
fn write_prefix(
162+
&mut self,
163+
path: impl std::fmt::Display,
164+
line_number: Option<u64>,
165+
sep: u8,
166+
) -> io::Result<()> {
159167
self.write_path(path)?;
160168
self.inner.write_all(&[sep])?;
161169
if let Some(line_number) = line_number {
@@ -186,7 +194,7 @@ impl<W: Write> PlainTextWriter<W> {
186194

187195
pub(crate) fn write_match_line(
188196
&mut self,
189-
path: &str,
197+
path: impl std::fmt::Display,
190198
line_number: Option<u64>,
191199
line: &[u8],
192200
matches: &[Match],
@@ -203,7 +211,7 @@ impl<W: Write> PlainTextWriter<W> {
203211

204212
pub(crate) fn write_plain_match_line(
205213
&mut self,
206-
path: &str,
214+
path: impl std::fmt::Display,
207215
line_number: Option<u64>,
208216
line: &[u8],
209217
) -> io::Result<()> {
@@ -215,7 +223,7 @@ impl<W: Write> PlainTextWriter<W> {
215223

216224
pub(crate) fn write_context_line(
217225
&mut self,
218-
path: &str,
226+
path: impl std::fmt::Display,
219227
line_number: Option<u64>,
220228
line: &[u8],
221229
) -> io::Result<()> {
@@ -234,20 +242,20 @@ impl<W: Write> PlainTextWriter<W> {
234242
self.inner.write_all(b"\n")
235243
}
236244

237-
pub(crate) fn write_file_match(&mut self, path: &str) -> io::Result<()> {
245+
pub(crate) fn write_file_match(&mut self, path: impl std::fmt::Display) -> io::Result<()> {
238246
self.write_path(path)?;
239247
self.inner.write_all(b"\n")
240248
}
241249

242-
pub(crate) fn write_binary_match(&mut self, path: &str) -> io::Result<()> {
250+
pub(crate) fn write_binary_match(&mut self, path: impl std::fmt::Display) -> io::Result<()> {
243251
self.inner.write_all(b"Binary file ")?;
244252
self.write_path(path)?;
245253
self.inner.write_all(b" matches\n")
246254
}
247255

248256
pub(crate) fn write_binary_quit_warning(
249257
&mut self,
250-
path: &str,
258+
path: impl std::fmt::Display,
251259
quit_byte: u8,
252260
offset: u64,
253261
) -> io::Result<()> {
@@ -266,23 +274,23 @@ impl<W: Write> PlainTextWriter<W> {
266274

267275
/// A grep match entry for JSON output.
268276
#[derive(Serialize)]
269-
pub(crate) struct GrepMatch<'a> {
270-
pub path: &'a str,
277+
pub(crate) struct GrepMatch<'a, P: Serialize> {
278+
pub path: P,
271279
#[serde(skip_serializing_if = "Option::is_none")]
272280
pub line_number: Option<u64>,
273281
pub text: &'a str,
274282
}
275283

276284
/// A file match entry for JSON output (used with -l flag).
277285
#[derive(Serialize)]
278-
pub(crate) struct GrepFileMatch<'a> {
279-
pub path: &'a str,
286+
pub(crate) struct GrepFileMatch<P: Serialize> {
287+
pub path: P,
280288
}
281289

282290
struct RelativizeOnce<'a> {
283291
relativizer: &'a RepoPathRelativizer,
284292
path: &'a RepoPathBuf,
285-
display: OnceCell<String>,
293+
display: OnceCell<RelativizedRepoPath<'a>>,
286294
}
287295

288296
impl<'a> RelativizeOnce<'a> {
@@ -294,10 +302,10 @@ impl<'a> RelativizeOnce<'a> {
294302
}
295303
}
296304

297-
fn as_str(&self) -> &str {
298-
self.display
299-
.get_or_init(|| self.relativizer.relativize(self.path))
300-
.as_str()
305+
fn display(&self) -> RelativizedRepoPath<'a> {
306+
*self
307+
.display
308+
.get_or_init(|| self.relativizer.relativized(self.path.as_repo_path()))
301309
}
302310
}
303311

@@ -461,7 +469,7 @@ impl<W: Write> Sink for StandardLineSink<'_, '_, W> {
461469
fn matched(&mut self, _searcher: &Searcher, mat: &SinkMatch<'_>) -> Result<bool, Self::Error> {
462470
let line_number = mat.line_number();
463471
let line = mat.bytes();
464-
let path = self.path.as_str();
472+
let path = self.path.display();
465473

466474
if self.invert_match || self.writer.match_style.is_empty() {
467475
self.writer
@@ -489,7 +497,7 @@ impl<W: Write> Sink for StandardLineSink<'_, '_, W> {
489497
context: &SinkContext<'_>,
490498
) -> Result<bool, Self::Error> {
491499
self.writer.write_context_line(
492-
self.path.as_str(),
500+
self.path.display(),
493501
context.line_number(),
494502
context.bytes(),
495503
)?;
@@ -519,7 +527,7 @@ impl<W: Write> Sink for StandardLineSink<'_, '_, W> {
519527
searcher.binary_detection().quit_byte(),
520528
) {
521529
self.writer.write_binary_quit_warning(
522-
self.path.as_str(),
530+
self.path.display(),
523531
quit_byte,
524532
binary_byte_offset,
525533
)?;
@@ -548,7 +556,7 @@ impl<W: Write> Sink for FileMatchLineSink<'_, '_, W> {
548556
type Error = io::Error;
549557

550558
fn matched(&mut self, _searcher: &Searcher, _mat: &SinkMatch<'_>) -> Result<bool, Self::Error> {
551-
self.writer.write_file_match(self.path.as_str())?;
559+
self.writer.write_file_match(self.path.display())?;
552560
self.has_match = true;
553561
Ok(false)
554562
}
@@ -1165,7 +1173,7 @@ pub(crate) fn grep_files_json(
11651173
}
11661174
if file_has_match {
11671175
json_out.write(&GrepFileMatch {
1168-
path: display_path.as_str(),
1176+
path: display_path.display(),
11691177
})?;
11701178
match_count += 1;
11711179
}
@@ -1220,7 +1228,7 @@ impl Sink for JsonWriteSink<'_, '_> {
12201228
let text = text.trim_end_matches('\n');
12211229

12221230
self.json_out.write(&GrepMatch {
1223-
path: self.path.as_str(),
1231+
path: self.path.display(),
12241232
line_number,
12251233
text,
12261234
})?;

‎eden/scm/lib/commands/commands/cmdstatus/src/lib.rs‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -246,13 +246,16 @@ pub fn run(ctx: ReqCtx<StatusOpts>, repo: &Repo, wc: &WorkingCopy) -> Result<u8>
246246
{
247247
lgr.warn(format!(
248248
"{}: invalid file type",
249-
relativizer.relativize(file)
249+
relativizer.relativized(file.as_repo_path())
250250
));
251251
}
252252
}
253253
Err(err) => {
254254
if !status.contains(file) {
255-
lgr.warn(format!("{}: {err}", relativizer.relativize(file)));
255+
lgr.warn(format!(
256+
"{}: {err}",
257+
relativizer.relativized(file.as_repo_path())
258+
));
256259
}
257260
}
258261
}

‎eden/scm/lib/commands/commands/cmdstatus/src/print.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ impl HgStatusPathRelativizer {
9595

9696
let out = match self.relativizer {
9797
Some(ref relativizer) => {
98-
let os_sep_path = relativizer.relativize(repo_path);
98+
let os_sep_path = relativizer.relativized(repo_path).to_string();
9999
match self.slash {
100100
PlatformDefault | BuggyRust => os_sep_path,
101101
ForwardSlash => {

0 commit comments

Comments
 (0)