Skip to content

Commit

Permalink
Optimize commit_file_diffs by running xdiff::diff_unified in a differ…
Browse files Browse the repository at this point in the history
…ent thread

Summary:
## This stack

Track and down methods that are holding up the reactor and optimize them.

## This diff

`xdiff::diff_unified` is synchronous and does a lot of work to format a diff. While we should optimize it, it is older code so that will realistically take a while. Running it on a separate, non-executor thread gets the job done.

Reviewed By: lmvasquezg

Differential Revision: D67025628

fbshipit-source-id: 1f946e8f7aabc0eb8f632a5724ed3a608ec6d877
  • Loading branch information
andreacampi authored and facebook-github-bot committed Dec 10, 2024
1 parent 3ebd46e commit 3334e9e
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 1 deletion.
4 changes: 3 additions & 1 deletion eden/mononoke/mononoke_api/src/changeset_path_diff.rs
Original file line number Diff line number Diff line change
Expand Up @@ -530,7 +530,9 @@ impl<R: MononokeRepo> ChangesetPathDiffContext<R> {
copy_info,
};
// The base is the target, so we diff in the opposite direction.
let raw_diff = xdiff::diff_unified(other_file, base_file, opts);
let raw_diff =
tokio::task::spawn_blocking(move || xdiff::diff_unified(other_file, base_file, opts))
.await?;
Ok(UnifiedDiff {
raw_diff,
is_binary,
Expand Down
7 changes: 7 additions & 0 deletions eden/mononoke/mononoke_api/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ use mononoke_types::ChangesetId;
use pushrebase::PushrebaseError;
use repo_authorization::AuthorizationError;
use thiserror::Error;
use tokio::task::JoinError;

#[derive(Clone, Debug)]
pub struct InternalError(Arc<Error>);
Expand Down Expand Up @@ -213,3 +214,9 @@ impl From<CommitCloudError> for MononokeError {
}
}
}

impl From<JoinError> for MononokeError {
fn from(e: JoinError) -> Self {
MononokeError::from(anyhow::Error::from(e))
}
}

0 comments on commit 3334e9e

Please sign in to comment.