mirror of
https://github.com/storytold/lightcraft.git
synced 2026-10-14 19:27:01 -04:00
Fix #367: import copies and verifies files on several threads
Import -> Copy (and Move, and Copy as DNG) handled one file after another on the import worker: copy, fsync, read back and hash, optionally decode and re-encode the raw as DNG. Every other CPU-heavy step of an import was already parallel (the probes, up to 8 threads), so with the probes cached by the review dialog's scan the transfer phase ran on one core. ImportJob::prepare_files now plans each file in order, as before (duplicates by path and content, sidecars, rename sequence numbers, folders, Kept notes) and collects the copies/moves, which then run on up to 8 threads (std::thread::scope; sequential on wasm32). Outcomes keep their places, so results, order, names and failure messages are those of a one-by-one import: - a file whose content matches a transfer still pending, or whose name could collide with one (same folder, same name family: IMG.jpg, IMG-1.jpg, img.dng), waits until that one has finished; - cancel stops before the next file; transfers not started leave no item; - a panic in one transfer fails that file instead of the batch; - Copy as DNG holds a memory-gate permit (6x the file size) while it runs. Copy verification and crash safety are unchanged (same copy_new / place). The desktop's import worker prepares batches of 2 x workers files (8..16, lightcraft_engine::import::batch_size) instead of 8, so a batch's slowest file holds back fewer threads; the browser build keeps 8. Measured (4-core shared VM, 200 procedural 3.7 MB JPEGs = 754 MB, warm page cache, throwaway benchmark not committed): - probe: ~80 ms CPU per file is the JPEG decode (DCT-scaled to 64 px), 1.2 ms the content hash; the dialog's scan: 4.4 s, 3.6 cores busy. - Copy, probes cached: copy+fsync+verify 1.59 s at 0.43 cores before, 0.94 s at 0.81 cores after. Copy, not cached: 6.45 s -> 5.51 s. - Add, probes cached: 2 ms prepare; nothing to parallelize. - commit of a batch on the UI thread (journal append + fsync): ~4 ms, flat for libraries of 0, 5k, 20k and 50k photos (~0.7 s for 1300). Copy as DNG could not be measured (no raw corpus here); it is the most CPU-heavy per-file step and was sequential. Test: copies_run_side_by_side_with_the_same_outcome (copies under the test fault Slow run on more than one thread; names, order and in-batch duplicates match a one-by-one copy; the outcome assertions also pass on the old code, the thread assertion fails there). Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01912WaK6Rmew3wxZuPyX7E8
This commit is contained in:
5 files changed
+340
-64
No files matched your search
+239
-56
@@ -365,7 +365,7 @@ fn probe_all(probe: Option<&crate::media::FileProbe>, paths: &[String], progress
|
||||
};
|
||||
#[cfg(not(target_arch = "wasm32"))]
|
||||
{
|
||||
let n = std::thread::available_parallelism().map(|n| n.get()).unwrap_or(4).clamp(1, 8).min(paths.len().max(1));
|
||||
let n = workers(paths.len());
|
||||
if n > 1 {
|
||||
let mut results: Vec<Option<Result<ProbeInfo, String>>> = vec![None; paths.len()];
|
||||
let next = std::sync::atomic::AtomicUsize::new(0);
|
||||
@@ -404,18 +404,16 @@ fn probe_all(probe: Option<&crate::media::FileProbe>, paths: &[String], progress
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// Copy `src` into `root`/`folders` as `name` (default: its own name), made unique with -1, -2…;
|
||||
/// returns the new path. The copy is verified ([`crate::import_move::copy_new`]: a new file,
|
||||
/// synced and checked against `probe_hash`, the content hash the probe computed — or compared
|
||||
/// byte for byte with the source when there is none); a bad one is removed and the photo
|
||||
/// reported as failed.
|
||||
fn copy_into(root: &Path, src: &str, folders: &[String], name: Option<&str>, probe_hash: Option<&str>) -> Result<String, String> {
|
||||
let dir = folders.iter().fold(root.to_path_buf(), |d, f| d.join(f));
|
||||
/// Copy `src` into `dir` as `name` (default: its own name), made unique with -1, -2…; returns the
|
||||
/// new path. The copy is verified ([`crate::import_move::copy_new`]: a new file, synced and
|
||||
/// checked against `probe_hash`, the content hash the probe computed — or compared byte for byte
|
||||
/// with the source when there is none); a bad one is removed and the photo reported as failed.
|
||||
fn copy_into(dir: &Path, src: &str, name: Option<&str>, probe_hash: Option<&str>) -> Result<String, String> {
|
||||
let name = name
|
||||
.map(str::to_string)
|
||||
.unwrap_or_else(|| Path::new(src).file_name().map(|n| n.to_string_lossy().to_string()).unwrap_or_else(|| "photo".into()));
|
||||
let expect = probe_hash.and_then(lightcraft_preview::Hash128::parse);
|
||||
crate::import_move::copy_new(Path::new(src), &dir, &name, expect).map(|p| p.to_string_lossy().to_string())
|
||||
crate::import_move::copy_new(Path::new(src), dir, &name, expect).map(|p| p.to_string_lossy().to_string())
|
||||
}
|
||||
|
||||
/// What a [`scan`] needs from the session, so it can run on another thread (a folder on a network
|
||||
@@ -699,8 +697,9 @@ impl ImportJob {
|
||||
}
|
||||
|
||||
/// The file-system half of importing `paths`: expand folders, skip known paths, probe, skip
|
||||
/// duplicates by content, read sidecars, copy or place (Move) the files. Nothing in the
|
||||
/// catalog changes. Stops before the next file once `cancel` is set.
|
||||
/// duplicates by content, read sidecars, copy or place (Move) the files — several at once,
|
||||
/// with the names and outcomes of one after another. Nothing in the catalog changes. Stops
|
||||
/// before the next file once `cancel` is set.
|
||||
pub fn prepare(&mut self, paths: &[String], cancel: &std::sync::atomic::AtomicBool) -> Prepared {
|
||||
let files = self.expand(paths);
|
||||
self.prepare_files(files, cancel)
|
||||
@@ -750,24 +749,39 @@ impl ImportJob {
|
||||
let probed: Vec<Result<ProbeInfo, String>> =
|
||||
cached.into_iter().map(|c| c.map(Ok).unwrap_or_else(|| fresh.next().unwrap_or_else(|| Err("not probed".into())))).collect();
|
||||
crate::memory::release();
|
||||
// Files are planned one by one, in order (duplicates, sidecars, names, folders); their
|
||||
// copies or moves — the slow part, reading and writing every byte and verifying it — are
|
||||
// collected and run on several threads (issue #367), then their outcomes take their places.
|
||||
let mut plan = Plan::default();
|
||||
for (path, info) in todo.into_iter().zip(probed) {
|
||||
if cancel.load(Relaxed) {
|
||||
break;
|
||||
}
|
||||
self.done.fetch_add(1, Relaxed);
|
||||
let mut info = match info {
|
||||
Ok(i) => i,
|
||||
Err(e) => {
|
||||
log::warn!("import {path}: {e}");
|
||||
out.items.push(PreparedItem::Failed(path, e));
|
||||
self.done.fetch_add(1, Relaxed);
|
||||
plan.slots.push(Slot::Item(PreparedItem::Failed(path, e)));
|
||||
continue;
|
||||
}
|
||||
};
|
||||
// a copy of a file whose transfer is still pending: a duplicate only if that one succeeds
|
||||
if !opts.local
|
||||
&& let Some(h) = &info.content_hash
|
||||
&& plan.transfers.iter().any(|t| t.info.content_hash.as_ref() == Some(h))
|
||||
{
|
||||
self.run_plan(&mut plan, &mut out, cancel);
|
||||
if cancel.load(Relaxed) {
|
||||
break;
|
||||
}
|
||||
}
|
||||
if let Some(h) = &info.content_hash
|
||||
&& !opts.local
|
||||
&& let Some(existing) = self.by_hash.get(h)
|
||||
{
|
||||
out.items.push(PreparedItem::Duplicate { path, existing: *existing, reason: "content", hash: Some(h.clone()) });
|
||||
self.done.fetch_add(1, Relaxed);
|
||||
plan.slots.push(Slot::Item(PreparedItem::Duplicate { path, existing: *existing, reason: "content", hash: Some(h.clone()) }));
|
||||
continue;
|
||||
}
|
||||
// the XMP sidecar (or a raw's embedded XMP), read before copying: its capture time files
|
||||
@@ -786,7 +800,6 @@ impl ImportJob {
|
||||
if info.captured.is_none() {
|
||||
info.captured = sidecar.as_ref().and_then(|sc| sc.captured.clone());
|
||||
}
|
||||
let mut placed = None;
|
||||
let stored = match (mode, &self.copy_root) {
|
||||
(ImportMode::Copy | ImportMode::Move, Some(root))
|
||||
if !crate::import_move::inside(Path::new(&path), root)
|
||||
@@ -803,37 +816,35 @@ impl ImportJob {
|
||||
n
|
||||
});
|
||||
let folders = opts.organize.folders(&q);
|
||||
if mode == ImportMode::Move && !crate::import_move::is_symlink(Path::new(&path)) {
|
||||
let dir = folders.iter().fold(root.to_path_buf(), |d, f| d.join(f));
|
||||
let name = name.unwrap_or(own);
|
||||
match crate::import_move::place(Path::new(&path), &dir, &name) {
|
||||
Ok(pl) => {
|
||||
let dst = pl.dst.to_string_lossy().to_string();
|
||||
placed = Some(pl);
|
||||
dst
|
||||
}
|
||||
Err(e) => {
|
||||
out.items.push(PreparedItem::Failed(path, e));
|
||||
continue;
|
||||
}
|
||||
let dir = folders.iter().fold(root.to_path_buf(), |d, f| d.join(f));
|
||||
// the name this file gets depends on what a pending transfer of a like-named file
|
||||
// into the same folder ends up with: that one goes first
|
||||
let as_named = name.as_deref().unwrap_or(if own.is_empty() { "photo" } else { own.as_str() });
|
||||
let family = (dir.to_string_lossy().to_lowercase(), name_family(as_named));
|
||||
if plan.transfers.iter().any(|t| t.family == family) {
|
||||
self.run_plan(&mut plan, &mut out, cancel);
|
||||
if cancel.load(Relaxed) {
|
||||
break;
|
||||
}
|
||||
}
|
||||
let how = if mode == ImportMode::Move && !crate::import_move::is_symlink(Path::new(&path)) {
|
||||
How::Place { name: name.unwrap_or(own) }
|
||||
} else {
|
||||
if mode == ImportMode::Move {
|
||||
let reason = "a symbolic link: the file it points to was copied, the link and its target stay";
|
||||
out.items.push(PreparedItem::Kept(Kept { path: path.clone(), reason: reason.into() }));
|
||||
plan.slots.push(Slot::Item(PreparedItem::Kept(Kept { path: path.clone(), reason: reason.into() })));
|
||||
}
|
||||
match copy_into(root, &path, &folders, name.as_deref(), info.content_hash.as_deref()) {
|
||||
Ok(p) => p,
|
||||
Err(e) => {
|
||||
out.items.push(PreparedItem::Failed(path, e));
|
||||
continue;
|
||||
}
|
||||
}
|
||||
}
|
||||
How::Copy { name }
|
||||
};
|
||||
// Copy as DNG: the copied raw becomes a DNG (the copy is ours to replace)
|
||||
let dng = opts.convert_dng && mode == ImportMode::Copy && raw && !info.format.eq_ignore_ascii_case("DNG");
|
||||
plan.slots.push(Slot::Transfer);
|
||||
plan.transfers.push(Transfer { path, info, sidecar, dir, how, dng, family });
|
||||
continue;
|
||||
}
|
||||
(ImportMode::Move, _) => {
|
||||
let reason = "already inside the destination or the library: added where it is";
|
||||
out.items.push(PreparedItem::Kept(Kept { path: path.clone(), reason: reason.into() }));
|
||||
plan.slots.push(Slot::Item(PreparedItem::Kept(Kept { path: path.clone(), reason: reason.into() })));
|
||||
path.clone()
|
||||
}
|
||||
_ => path.clone(),
|
||||
@@ -841,27 +852,199 @@ impl ImportJob {
|
||||
if let Some(h) = &info.content_hash {
|
||||
self.by_hash.insert(h.clone(), None);
|
||||
}
|
||||
// Copy as DNG: the copied raw becomes a DNG (the copy is ours to replace)
|
||||
let mut stored = stored;
|
||||
if opts.convert_dng
|
||||
&& mode == ImportMode::Copy
|
||||
&& stored != path
|
||||
&& info.kind == MediaKind::Raw
|
||||
&& !info.format.eq_ignore_ascii_case("DNG")
|
||||
{
|
||||
match crate::cmd::convert::write_dng_with(self.file_bytes.as_ref(), &stored, String::new()) {
|
||||
Ok(dng) => {
|
||||
let _ = std::fs::remove_file(&stored);
|
||||
stored = dng;
|
||||
info.format = "DNG".into();
|
||||
}
|
||||
Err(e) => log::warn!("import {path}: copy as DNG: {e}"),
|
||||
}
|
||||
}
|
||||
out.items.push(PreparedItem::Ready(Box::new(ReadyFile { path, stored, info, sidecar, placed })));
|
||||
self.done.fetch_add(1, Relaxed);
|
||||
plan.slots.push(Slot::Item(PreparedItem::Ready(Box::new(ReadyFile { path, stored, info, sidecar, placed: None }))));
|
||||
}
|
||||
self.run_plan(&mut plan, &mut out, cancel);
|
||||
out
|
||||
}
|
||||
|
||||
/// Run the transfers `plan` collected (on several threads) and append its items to `out` in
|
||||
/// file order. Transfers not started because `cancel` was set leave no item.
|
||||
fn run_plan(&mut self, plan: &mut Plan, out: &mut Prepared, cancel: &std::sync::atomic::AtomicBool) {
|
||||
let mut done = run_transfers(std::mem::take(&mut plan.transfers), self.file_bytes.as_ref(), cancel, &self.done).into_iter();
|
||||
for slot in std::mem::take(&mut plan.slots) {
|
||||
let item = match slot {
|
||||
Slot::Item(it) => it,
|
||||
Slot::Transfer => match done.next().flatten() {
|
||||
Some(it) => it,
|
||||
None => continue,
|
||||
},
|
||||
};
|
||||
if let PreparedItem::Ready(r) = &item
|
||||
&& let Some(h) = &r.info.content_hash
|
||||
{
|
||||
self.by_hash.insert(h.clone(), None);
|
||||
}
|
||||
out.items.push(item);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// A batch's items in file order while its transfers are pending ([`ImportJob::run_plan`]).
|
||||
#[derive(Default)]
|
||||
struct Plan {
|
||||
slots: Vec<Slot>,
|
||||
transfers: Vec<Transfer>,
|
||||
}
|
||||
|
||||
enum Slot {
|
||||
Item(PreparedItem),
|
||||
/// The outcome of the next of [`Plan::transfers`].
|
||||
Transfer,
|
||||
}
|
||||
|
||||
/// A file to copy or move into `dir`, planned in order and run on any thread ([`run_transfer`]).
|
||||
struct Transfer {
|
||||
path: String,
|
||||
info: ProbeInfo,
|
||||
sidecar: Option<crate::sidecar::SidecarData>,
|
||||
dir: PathBuf,
|
||||
how: How,
|
||||
/// Copy as DNG.
|
||||
dng: bool,
|
||||
/// The folder (lowercase) and [`name_family`]: transfers of one family can take each other's
|
||||
/// names, so they never run at the same time.
|
||||
family: (String, String),
|
||||
}
|
||||
|
||||
enum How {
|
||||
/// A verified copy, under `name` (default: the file's own).
|
||||
Copy { name: Option<String> },
|
||||
/// Move: placed (linked or copied) without touching the source.
|
||||
Place { name: String },
|
||||
}
|
||||
|
||||
/// The names a file called `name` could end up with, as one key: the transfers number a taken name
|
||||
/// `IMG-1.JPG`, `IMG-2.JPG`…, and Copy as DNG names its DNG `IMG.dng`, `IMG-2.dng`…, so `IMG.JPG`,
|
||||
/// `IMG-1.jpg` and `img.dng` are all `img` (case-insensitive: so are some file systems).
|
||||
fn name_family(name: &str) -> String {
|
||||
let mut stem = match name.rsplit_once('.') {
|
||||
Some((s, _)) if !s.is_empty() => s,
|
||||
_ => name,
|
||||
};
|
||||
while let Some((head, tail)) = stem.rsplit_once('-')
|
||||
&& !head.is_empty()
|
||||
&& !tail.is_empty()
|
||||
&& tail.bytes().all(|b| b.is_ascii_digit())
|
||||
{
|
||||
stem = head;
|
||||
}
|
||||
stem.to_lowercase()
|
||||
}
|
||||
|
||||
/// Threads for `jobs` file reads (probes, copies) at once.
|
||||
#[cfg(not(target_arch = "wasm32"))]
|
||||
fn workers(jobs: usize) -> usize {
|
||||
std::thread::available_parallelism().map(|n| n.get()).unwrap_or(4).clamp(1, 8).min(jobs.max(1))
|
||||
}
|
||||
|
||||
/// Files per batch for a caller that prepares an import in batches ([`ImportJob::prepare_files`]):
|
||||
/// enough to keep every probe and copy thread busy (a batch waits for its slowest file), few
|
||||
/// enough that each batch soon joins the catalog.
|
||||
pub fn batch_size() -> usize {
|
||||
#[cfg(not(target_arch = "wasm32"))]
|
||||
{
|
||||
workers(usize::MAX).saturating_mul(2).clamp(8, 16)
|
||||
}
|
||||
#[cfg(target_arch = "wasm32")]
|
||||
{
|
||||
8
|
||||
}
|
||||
}
|
||||
|
||||
/// Run `transfers` (several at once where threads exist); their outcomes in order, `None` for one
|
||||
/// not started because `cancel` was set. `done` counts each one finished.
|
||||
fn run_transfers(
|
||||
transfers: Vec<Transfer>,
|
||||
file_bytes: Option<&crate::merge::ByteReader>,
|
||||
cancel: &std::sync::atomic::AtomicBool,
|
||||
done: &std::sync::atomic::AtomicUsize,
|
||||
) -> Vec<Option<PreparedItem>> {
|
||||
use std::sync::atomic::Ordering::Relaxed;
|
||||
let one = |t: Transfer| {
|
||||
if cancel.load(Relaxed) {
|
||||
return None;
|
||||
}
|
||||
let path = t.path.clone();
|
||||
// last-resort guard: a panic is this file's failure, not the import's
|
||||
let item = crate::guard::catch("import", || run_transfer(t, file_bytes)).unwrap_or_else(|e| PreparedItem::Failed(path, e));
|
||||
done.fetch_add(1, Relaxed);
|
||||
Some(item)
|
||||
};
|
||||
#[cfg(not(target_arch = "wasm32"))]
|
||||
{
|
||||
let n = workers(transfers.len());
|
||||
if n > 1 {
|
||||
let mut results: Vec<Option<PreparedItem>> = std::iter::repeat_with(|| None).take(transfers.len()).collect();
|
||||
let queue = std::sync::Mutex::new(transfers.into_iter().enumerate());
|
||||
let out = std::sync::Mutex::new(&mut results);
|
||||
// (the tests' injected faults hold for the workers too)
|
||||
#[cfg(test)]
|
||||
let fault = crate::import_move::injected_fault();
|
||||
let work = &|| {
|
||||
#[cfg(test)]
|
||||
crate::import_move::inject(fault);
|
||||
loop {
|
||||
let next = queue.lock().unwrap_or_else(std::sync::PoisonError::into_inner).next();
|
||||
let Some((i, t)) = next else { break };
|
||||
let r = one(t);
|
||||
if let Some(slot) = out.lock().unwrap_or_else(std::sync::PoisonError::into_inner).get_mut(i) {
|
||||
*slot = r;
|
||||
}
|
||||
}
|
||||
};
|
||||
std::thread::scope(|sc| {
|
||||
let handles: Vec<_> =
|
||||
(0..n).filter_map(|i| std::thread::Builder::new().name(format!("lc-import-copy-{i}")).spawn_scoped(sc, work).ok()).collect();
|
||||
if handles.is_empty() {
|
||||
// no thread to be had: copy here, one after another
|
||||
work();
|
||||
}
|
||||
for h in handles {
|
||||
if h.join().is_err() {
|
||||
log::error!("import: a copy worker stopped unexpectedly");
|
||||
}
|
||||
}
|
||||
});
|
||||
return results;
|
||||
}
|
||||
}
|
||||
transfers.into_iter().map(one).collect()
|
||||
}
|
||||
|
||||
/// Copy or move one planned file (and Copy as DNG); its item.
|
||||
fn run_transfer(t: Transfer, file_bytes: Option<&crate::merge::ByteReader>) -> PreparedItem {
|
||||
let Transfer { path, mut info, sidecar, dir, how, dng, .. } = t;
|
||||
let mut placed = None;
|
||||
let mut stored = match how {
|
||||
How::Place { name } => match crate::import_move::place(Path::new(&path), &dir, &name) {
|
||||
Ok(pl) => {
|
||||
let dst = pl.dst.to_string_lossy().to_string();
|
||||
placed = Some(pl);
|
||||
dst
|
||||
}
|
||||
Err(e) => return PreparedItem::Failed(path, e),
|
||||
},
|
||||
How::Copy { name } => match copy_into(&dir, &path, name.as_deref(), info.content_hash.as_deref()) {
|
||||
Ok(p) => p,
|
||||
Err(e) => return PreparedItem::Failed(path, e),
|
||||
},
|
||||
};
|
||||
if dng {
|
||||
// decoding the raw twice and encoding it takes several times the file's size in memory
|
||||
let weight = usize::try_from(info.file_size).unwrap_or(usize::MAX).saturating_mul(6);
|
||||
let _permit = crate::memory::work_gate().acquire(weight);
|
||||
match crate::cmd::convert::write_dng_with(file_bytes, &stored, String::new()) {
|
||||
Ok(dng) => {
|
||||
let _ = std::fs::remove_file(&stored);
|
||||
stored = dng;
|
||||
info.format = "DNG".into();
|
||||
}
|
||||
Err(e) => log::warn!("import {path}: copy as DNG: {e}"),
|
||||
}
|
||||
}
|
||||
PreparedItem::Ready(Box::new(ReadyFile { path, stored, info, sidecar, placed }))
|
||||
}
|
||||
|
||||
/// The catalog half of an import: add the photos [`ImportJob::prepare`] readied (one undoable op;
|
||||
|
||||
@@ -56,6 +56,9 @@ pub(crate) enum Fault {
|
||||
Corrupt,
|
||||
/// Removing the source fails (as on a read-only card).
|
||||
FailRemove,
|
||||
/// Each copy takes a little longer and notes the thread it ran on ([`copy_threads`]), to see
|
||||
/// an import's copies run side by side.
|
||||
Slow,
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
@@ -69,6 +72,21 @@ pub(crate) fn inject(f: Fault) {
|
||||
FAULT.with(|c| c.set(f));
|
||||
}
|
||||
|
||||
/// The calling thread's injected fault (tests only), for the workers it starts to inherit.
|
||||
#[cfg(test)]
|
||||
pub(crate) fn injected_fault() -> Fault {
|
||||
FAULT.with(|c| c.get())
|
||||
}
|
||||
|
||||
/// The threads copies made under [`Fault::Slow`] ran on (tests only).
|
||||
#[cfg(test)]
|
||||
static COPY_THREADS: std::sync::Mutex<Vec<std::thread::ThreadId>> = std::sync::Mutex::new(Vec::new());
|
||||
|
||||
#[cfg(test)]
|
||||
pub(crate) fn copy_threads() -> Vec<std::thread::ThreadId> {
|
||||
COPY_THREADS.lock().unwrap_or_else(std::sync::PoisonError::into_inner).clone()
|
||||
}
|
||||
|
||||
fn injected(f: Fault) -> bool {
|
||||
#[cfg(test)]
|
||||
{
|
||||
@@ -284,6 +302,11 @@ pub(crate) fn copy_verified(src: &Path, dst: &Path, expect: Option<lightcraft_pr
|
||||
return Err(io::Error::other("write failed (injected)"));
|
||||
}
|
||||
io::copy(&mut inp, &mut out)?;
|
||||
#[cfg(test)]
|
||||
if injected(Fault::Slow) {
|
||||
COPY_THREADS.lock().unwrap_or_else(std::sync::PoisonError::into_inner).push(std::thread::current().id());
|
||||
std::thread::sleep(std::time::Duration::from_millis(30));
|
||||
}
|
||||
if injected(Fault::Corrupt) {
|
||||
out.write_all(b"!")?;
|
||||
}
|
||||
|
||||
@@ -433,3 +433,70 @@ fn write_policy_timing() {
|
||||
println!("copy {n} × {} MB: byte compare with the source {bytewise:?}, probe hash {hashed:?}", size >> 20);
|
||||
let _ = std::fs::remove_dir_all(&base);
|
||||
}
|
||||
|
||||
/// Issue #367: Import → Copy wrote and verified one file after another on one thread. The copies
|
||||
/// now run side by side, with the names, order, duplicates and reports a one-by-one copy gives:
|
||||
/// like-named files take -1, -2… in file order (also against a file already there and a literal
|
||||
/// `IMG_1-1`), and a file with the content of one copied earlier in the batch is its duplicate
|
||||
/// (also while that copy is still under way).
|
||||
#[test]
|
||||
fn copies_run_side_by_side_with_the_same_outcome() {
|
||||
let base = temp_dir("parallel-copy");
|
||||
let (card, dest) = (base.join("card"), base.join("out"));
|
||||
write_png(&card.join("a/IMG_1.png"), 1);
|
||||
write_png(&card.join("a/IMG_2.png"), 2);
|
||||
write_png(&card.join("b/IMG_1.png"), 3);
|
||||
write_png(&card.join("b/IMG_1-1.png"), 4);
|
||||
write_png(&card.join("c/IMG_1.png"), 5);
|
||||
for i in 0..8u8 {
|
||||
write_png(&card.join(format!("d/p{i}.png")), 10 + i);
|
||||
}
|
||||
// the content of a file whose copy is still under way
|
||||
std::fs::create_dir_all(card.join("e")).unwrap();
|
||||
std::fs::copy(card.join("d/p3.png"), card.join("e/dup.png")).unwrap();
|
||||
std::fs::create_dir_all(&dest).unwrap();
|
||||
std::fs::write(dest.join("IMG_2.png"), b"someone else's file").unwrap();
|
||||
|
||||
let mut s = session();
|
||||
inject(Fault::Slow);
|
||||
let r = s
|
||||
.execute(
|
||||
"library.import",
|
||||
&json!({"paths": [card.to_string_lossy()], "mode": "copy", "destination": dest.to_string_lossy(), "organize": "flat"}),
|
||||
)
|
||||
.unwrap();
|
||||
inject(Fault::None);
|
||||
let threads: std::collections::HashSet<_> = crate::import_move::copy_threads().into_iter().collect();
|
||||
if std::thread::available_parallelism().map_or(1, |n| n.get()) > 1 {
|
||||
assert!(threads.len() > 1, "copies ran on {} thread(s)", threads.len());
|
||||
}
|
||||
|
||||
assert_eq!(len(&r, "imported"), 13, "{r}");
|
||||
assert_eq!(len(&r, "failed"), 0, "{r}");
|
||||
assert_eq!(len(&r, "duplicates"), 1, "{r}");
|
||||
assert_eq!(r["duplicates"][0]["reason"], "content");
|
||||
assert!(r["duplicates"][0]["path"].as_str().unwrap().ends_with("dup.png"), "{r}");
|
||||
// the order of a one-by-one copy (files in name order), each verified
|
||||
let expect = [
|
||||
("a/IMG_1.png", "IMG_1.png"),
|
||||
("a/IMG_2.png", "IMG_2-1.png"),
|
||||
("b/IMG_1-1.png", "IMG_1-1.png"),
|
||||
("b/IMG_1.png", "IMG_1-2.png"),
|
||||
("c/IMG_1.png", "IMG_1-3.png"),
|
||||
];
|
||||
for (src, dst) in expect {
|
||||
assert_eq!(std::fs::read(dest.join(dst)).unwrap(), std::fs::read(card.join(src)).unwrap(), "{src} → {dst}");
|
||||
}
|
||||
for i in 0..8 {
|
||||
assert_eq!(std::fs::read(dest.join(format!("p{i}.png"))).unwrap(), std::fs::read(card.join(format!("d/p{i}.png"))).unwrap());
|
||||
}
|
||||
assert_eq!(std::fs::read(dest.join("IMG_2.png")).unwrap(), b"someone else's file");
|
||||
assert_eq!(files_under(&dest).len(), 14, "{:?}", files_under(&dest));
|
||||
// the duplicate names the photo copied from the same content (d/p3); ids follow the file order
|
||||
let ids: Vec<u64> = r["imported"].as_array().unwrap().iter().map(|v| v.as_u64().unwrap()).collect();
|
||||
assert!(ids.windows(2).all(|w| w[0] < w[1]), "{ids:?}");
|
||||
assert_eq!(r["duplicates"][0]["existing"], ids[8], "{r}");
|
||||
let first = s.catalog.photo(lightcraft_catalog::PhotoId(ids[0])).unwrap();
|
||||
assert_eq!(first.file_name, "IMG_1.png");
|
||||
let _ = std::fs::remove_dir_all(&base);
|
||||
}
|
||||
@@ -1635,8 +1635,9 @@ mod tests {
|
||||
let _ = std::fs::remove_dir_all(&base);
|
||||
let (src, dest) = (base.join("card"), base.join("out"));
|
||||
std::fs::create_dir_all(&src).unwrap();
|
||||
for i in 0..10u8 {
|
||||
let img = lightcraft_raster::Rgba8 { width: 8, height: 8, data: vec![[i * 20, 3, 9, 255]; 64] };
|
||||
// more files than a batch holds (see `batch_size`)
|
||||
for i in 0..20u8 {
|
||||
let img = lightcraft_raster::Rgba8 { width: 8, height: 8, data: vec![[i * 12, 3, 9, 255]; 64] };
|
||||
let png = lightcraft_codecs::encode_png(&lightcraft_codecs::EncodeImage::rgba8(&img), &lightcraft_codecs::EncodeMeta::default()).unwrap();
|
||||
std::fs::write(src.join(format!("IMG_{i:02}.png")), png).unwrap();
|
||||
}
|
||||
@@ -1651,7 +1652,7 @@ mod tests {
|
||||
h.settle(SETTLE);
|
||||
h.step_until(SETTLE, |h| matches!(h.app.ui.dialog, Some(crate::state::Dialog::Import { .. })));
|
||||
let Some(crate::state::Dialog::Import { opts }) = &h.app.ui.dialog else { panic!("no import review") };
|
||||
assert_eq!(opts.candidates.len(), 10);
|
||||
assert_eq!(opts.candidates.len(), 20);
|
||||
for id in ["button:importCopy", "button:importDest"] {
|
||||
let r = h.request("ui.clickWidget", json!({"id": id}), t);
|
||||
assert_eq!(r["ok"], true, "{id}: {r}");
|
||||
@@ -1670,8 +1671,8 @@ mod tests {
|
||||
assert!(h.app.import.is_none(), "finished");
|
||||
let mut names: Vec<String> = std::fs::read_dir(&dest).unwrap().flatten().map(|e| e.file_name().to_string_lossy().to_string()).collect();
|
||||
names.sort();
|
||||
let want: Vec<String> = (1..=10).map(|i| format!("Trip-{i:02}.png")).collect();
|
||||
assert_eq!(names, want, "numbered across batches of {}", crate::import::BATCH);
|
||||
let want: Vec<String> = (1..=20).map(|i| format!("Trip-{i:02}.png")).collect();
|
||||
assert_eq!(names, want, "numbered across batches of {}", lightcraft_engine::import::batch_size());
|
||||
let _ = std::fs::remove_dir_all(&base);
|
||||
}
|
||||
|
||||
|
||||
@@ -18,8 +18,10 @@ use crate::render::Slot;
|
||||
use crate::theme::Tokens;
|
||||
use crate::widgets::register;
|
||||
|
||||
/// Files per batch (each batch joins the catalog as it is ready, so the progress window updates).
|
||||
pub(crate) const BATCH: usize = 8;
|
||||
/// Files per batch in the browser build (each batch joins the catalog as it is ready, so the
|
||||
/// progress window updates); the desktop's worker uses [`lightcraft_engine::import::batch_size`].
|
||||
#[cfg(target_arch = "wasm32")]
|
||||
const BATCH: usize = 8;
|
||||
|
||||
/// The review dialog's first size (points). It can be resized; the photo grid takes the height.
|
||||
pub(crate) const DIALOG_SIZE: [f32; 2] = [960.0, 720.0];
|
||||
@@ -273,7 +275,7 @@ impl ImportRun {
|
||||
let ctx = ctx.clone();
|
||||
let work = move || {
|
||||
let files = job.expand(&queue);
|
||||
for chunk in files.chunks(BATCH) {
|
||||
for chunk in files.chunks(lightcraft_engine::import::batch_size()) {
|
||||
if cancel.load(Ordering::Relaxed) {
|
||||
break;
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user