This is an archive of the discontinued Mercurial Phabricator instance.

rust/hg-core/src/dirstate/status.rs
68	note: took me some time to realize that skipping the full directory if there is a `.hg` make sense. Because we detected a another repository and that the `wc` is part of that repository. Maybe follow up with a documentation update that clarify this.
74	Silly question: would it make sense to sort the filename before the loop ? It might result is less bytes to move around since the entry will not be attached yet.

Alphare marked an inline comment as done.Feb 6 2020, 10:02 AM

Alphare added inline comments.

rust/hg-core/src/dirstate/status.rs
74	I'm not entirely sure I understand your question.

Alphare edited parent revisions, added: D7925: rust-matchers: add `IgnoreMatcher`; removed: D7926: rust-dependencies: update rayon.Feb 6 2020, 10:15 AM

Alphare added a child revision: D8086: rust-status: refactor options into a `StatusOptions` struct.

Alphare removed a child revision: D7928: rust-status: add function for sequential traversal of the working directory.

Alphare updated this revision to Diff 19942.

marmoute added inline comments.Feb 6 2020, 2:46 PM

rust/hg-core/src/dirstate/status.rs
74	If I understand you code correctly you currently do: filenames = listdir() result = [(f, entry(f)) for f in filenames] result.sort() Would it make sense to do: filenames = listdir() filenames.sort() result = [(f, entry(f)) for f in filenames]

Alphare added inline comments.Feb 10 2020, 11:11 AM

rust/hg-core/src/dirstate/status.rs
74	std::fs::read_dir returns an iterator, so I would have to collect it first, unsure if that would be more expensive... and also, a `Vec` of heap-allocated elements being just an array of pointers I'm not convinced that would help. Maybe we can do some benchmarking further down the line.

kevincox accepted this revision.Feb 12 2020, 5:09 AM

kevincox added inline comments.

rust/hg-core/src/dirstate/status.rs
76	It would be more clear to do `.sort_by_key(\|e\| e.0)` And I don't think stability matters here so you could use `sort_unstable_by_key`.

Alphare marked an inline comment as done.Feb 12 2020, 8:53 AM

Alphare added inline comments.Feb 12 2020, 9:02 AM

rust/hg-core/src/dirstate/status.rs
76	I didn't realize that `sort_unstable_by_key` needed to return a `T` and not a `&T`, that forces a `clone()`. I'm not sure that matters, we'll go with the shortest code and see if that shows up in profiling later.

kevincox added inline comments.Feb 12 2020, 11:02 AM

rust/hg-core/src/dirstate/status.rs
76	`sort_unstable_by_key` doesn't require the function to return a T at all. It just needs to return an `Ord` type. Note that `&Ord` is `Ord` automatically. https://doc.rust-lang.org/std/primitive.slice.html#method.sort_unstable_by_key

Alphare updated this revision to Diff 20183.Feb 13 2020, 12:36 PM

Alphare added a commit: rHG0d97bcb3cee9: rust-status: add util for listing a directory.Mar 11 2020, 12:23 PM

This revision was not accepted when it landed; it landed in state Needs Review.

Closed by commit rHG0d97bcb3cee9: rust-status: add util for listing a directory (authored by Alphare). · Explain Why

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

			Path	Packages
M			rust/hg-core/src/dirstate/status.rs (31 lines)

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	19416		Jan 17 2020, 10:33 AM	★	★
Diff 2	19942		Feb 6 2020, 10:15 AM	★	★
Diff 3	20183		Feb 13 2020, 12:36 PM	★	★
Diff 4	20716	rHG0d97bcb3cee9e608481967d8cb67f472cb867304	Jan 17 2020, 9:37 AM	★	★

Status	Author	Revision
Closed	Alphare	D8253 hg-core: add function timing information
Closed	Alphare	D8252 rust: add logging utils
Closed	Alphare	D8251 rust-status: traverse working directory in parallel
Closed	Alphare	D8250 rust-status: wrap `stat_dmap_entries` to ease profiling
Closed	Alphare	D8249 rust-status: refactor handling of unknown files
Closed	Alphare	D8215 rust-status: move to recursive traversal to prepare for parallel traversal
Closed	Alphare	D8213 rust-pathauditor: use interior mutability for use in multi-threaded contexts
Closed	Alphare	D8212 rust-pathauditor: actually populate the `audited_dirs` cache
Closed	Alphare	D8211 rust-pathauditor: actually split Windows shortname aliases at `~`
Closed	Alphare	D8226 debuginstall: add entry about re2 Rust bindings when applicable
Closed	Alphare	D8225 rust-cpython: add `debug` module to expose debug information to Python
Closed	Alphare	D7931 rust-status: use bare hg status fastpath from Python
Closed	Alphare	D7930 rust-status: update rust-cpython bridge to account for the changes in core
Closed	Alphare	D7929 rust-status: add bare `hg status` support in hg-core
Closed	Alphare	D7928 rust-status: add function for sequential traversal of the working directory
Closed	Alphare	D8088 rust-status: add missing variants to `Dispatch` enum
Closed	Alphare	D8087 rust-status: rename `StatusResult` to `DirstateStatus`
Closed	Alphare	D8086 rust-status: refactor options into a `StatusOptions` struct
Closed	Alphare	D7927 rust-status: add util for listing a directory
Closed	Alphare	D7926 rust-dependencies: update rayon
Closed	Alphare	D7925 rust-matchers: add `IgnoreMatcher`
Closed	Alphare	D7924 rust-matchers: add `build_regex_match` function
Closed	Alphare	D7923 rust-matchers: add functions to get roots, dirs and parents from patterns
Closed	Alphare	D7922 rust-matchers: add function to generate a regex matcher function
Closed	Alphare	D8224 rust-format: cleanup ancestors.rs to make rustfmt happy

Diff 20716

rust/hg-core/src/dirstate/status.rs

	// status.rs			// status.rs
	//			//
	// Copyright 2019 Raphaël Gomès <rgomes@octobus.net>			// Copyright 2019 Raphaël Gomès <rgomes@octobus.net>
	//			//
	// This software may be used and distributed according to the terms of the			// This software may be used and distributed according to the terms of the
	// GNU General Public License version 2 or any later version.			// GNU General Public License version 2 or any later version.

	//! Rust implementation of dirstate.status (dirstate.py).			//! Rust implementation of dirstate.status (dirstate.py).
	//! It is currently missing a lot of functionality compared to the Python one			//! It is currently missing a lot of functionality compared to the Python one
	//! and will only be triggered in narrow cases.			//! and will only be triggered in narrow cases.

	use crate::{			use crate::{
	dirstate::SIZE_FROM_OTHER_PARENT,			dirstate::SIZE_FROM_OTHER_PARENT,
	matchers::Matcher,			matchers::Matcher,
	utils::{			utils::{
	files::HgMetadata,			files::HgMetadata,
	hg_path::{hg_path_to_path_buf, HgPath},			hg_path::{
				hg_path_to_path_buf, os_string_to_hg_path_buf, HgPath, HgPathBuf,
				},
	},			},
	CopyMap, DirstateEntry, DirstateMap, EntryState,			CopyMap, DirstateEntry, DirstateMap, EntryState,
	};			};
	use rayon::prelude::*;			use rayon::prelude::*;
	use std::collections::HashSet;			use std::collections::HashSet;
				use std::fs::{read_dir, DirEntry};
	use std::path::Path;			use std::path::Path;

	/// Marker enum used to dispatch new status entries into the right collections.			/// Marker enum used to dispatch new status entries into the right collections.
	/// Is similar to `crate::EntryState`, but represents the transient state of			/// Is similar to `crate::EntryState`, but represents the transient state of
	/// entries during the lifetime of a command.			/// entries during the lifetime of a command.
	enum Dispatch {			enum Dispatch {
	Unsure,			Unsure,
	Modified,			Modified,
	/// changes. (issue2608)			/// changes. (issue2608)
	/// TODO I haven't found a way of having `b` be `Into<i32>`, since `From<u64>`			/// TODO I haven't found a way of having `b` be `Into<i32>`, since `From<u64>`
	/// is not defined for `i32`, and there is no `As` trait. This forces the			/// is not defined for `i32`, and there is no `As` trait. This forces the
	/// caller to cast `b` as `i32`.			/// caller to cast `b` as `i32`.
	fn mod_compare(a: i32, b: i32) -> bool {			fn mod_compare(a: i32, b: i32) -> bool {
	a & i32::max_value() != b & i32::max_value()			a & i32::max_value() != b & i32::max_value()
	}			}

				/// Return a sorted list containing information about the entries
				/// in the directory.
				///
				/// * `skip_dot_hg` - Return an empty vec if `path` contains a `.hg` directory
				fn list_directory(
				path: impl AsRef<Path>,
				skip_dot_hg: bool,
				) -> std::io::Result<Vec<(HgPathBuf, DirEntry)>> {
				let mut results = vec![];
				let entries = read_dir(path.as_ref())?;

				for entry in entries {
				let entry = entry?;
				let filename = os_string_to_hg_path_buf(entry.file_name())?;
				let file_type = entry.file_type()?;
				marmouteUnsubmitted Done note: took me some time to realize that skipping the full directory if there is a `.hg` make sense. Because we detected a another repository and that the `wc` is part of that repository. Maybe follow up with a documentation update that clarify this. marmoute: note: took me some time to realize that skipping the full directory if there is a `.hg` make…
				if skip_dot_hg && filename.as_bytes() == b".hg" && file_type.is_dir() {
				return Ok(vec![]);
				} else {
				results.push((HgPathBuf::from(filename), entry))
				}
				}
				marmouteUnsubmitted Not Done Silly question: would it make sense to sort the filename before the loop ? It might result is less bytes to move around since the entry will not be attached yet. marmoute: Silly question: would it make sense to sort the filename before the loop ? It might result is…
				AlphareAuthorUnsubmitted Done I'm not entirely sure I understand your question. Alphare: I'm not entirely sure I understand your question.
				marmouteUnsubmitted Not Done If I understand you code correctly you currently do: filenames = listdir() result = [(f, entry(f)) for f in filenames] result.sort() Would it make sense to do: filenames = listdir() filenames.sort() result = [(f, entry(f)) for f in filenames] marmoute: If I understand you code correctly you currently do: ``` filenames = listdir() result = [(f…
				AlphareAuthorUnsubmitted Done std::fs::read_dir returns an iterator, so I would have to collect it first, unsure if that would be more expensive... and also, a `Vec` of heap-allocated elements being just an array of pointers I'm not convinced that would help. Maybe we can do some benchmarking further down the line. Alphare: [[ https://doc.rust-lang.org/nightly/std/fs/fn.read_dir.html \| std::fs::read_dir ]] returns an…

				results.sort_unstable_by_key(\|e\| e.0.clone());
				kevincoxUnsubmitted Done It would be more clear to do `.sort_by_key(\|e\| e.0)` And I don't think stability matters here so you could use `sort_unstable_by_key`. kevincox: It would be more clear to do `.sort_by_key(\|e\| e.0)` And I don't think stability matters here…
				AlphareAuthorUnsubmitted Done I didn't realize that `sort_unstable_by_key` needed to return a `T` and not a `&T`, that forces a `clone()`. I'm not sure that matters, we'll go with the shortest code and see if that shows up in profiling later. Alphare: I didn't realize that `sort_unstable_by_key` needed to return a `T` and not a `&T`, that forces…
				kevincoxUnsubmitted Not Done `sort_unstable_by_key` doesn't require the function to return a T at all. It just needs to return an `Ord` type. Note that `&Ord` is `Ord` automatically. https://doc.rust-lang.org/std/primitive.slice.html#method.sort_unstable_by_key kevincox: `sort_unstable_by_key` doesn't require the function to return a T at all. It just needs to…
				Ok(results)
				}

	/// The file corresponding to the dirstate entry was found on the filesystem.			/// The file corresponding to the dirstate entry was found on the filesystem.
	fn dispatch_found(			fn dispatch_found(
	filename: impl AsRef<HgPath>,			filename: impl AsRef<HgPath>,
	entry: DirstateEntry,			entry: DirstateEntry,
	metadata: HgMetadata,			metadata: HgMetadata,
	copy_map: &CopyMap,			copy_map: &CopyMap,
	check_exec: bool,			check_exec: bool,
	list_clean: bool,			list_clean: bool,

This is an archive of the discontinued Mercurial Phabricator instance.

rust-status: add util for listing a directoryClosedPublic

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 20716

rust/hg-core/src/dirstate/status.rs

rust-status: add util for listing a directory
ClosedPublic

Revision Contents
Changeset List