Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

replace manual time convertions with std ones, comptime time format parsing #132521

Merged
merged 2 commits into from
Nov 4, 2024
Merged

replace manual time convertions with std ones, comptime time format parsing #132521

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
be3a635
replace manual time convertions with std ones
klensy Nov 2, 2024
bc1d8e7
make time format parsing compiletime
klensy Nov 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion compiler/rustc_driver_impl/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ rustc_trait_selection = { path = "../rustc_trait_selection" }
rustc_ty_utils = { path = "../rustc_ty_utils" }
serde_json = "1.0.59"
shlex = "1.0"
time = { version = "0.3.36", default-features = false, features = ["alloc", "formatting", "parsing", "macros"] }
time = { version = "0.3.36", default-features = false, features = ["alloc", "formatting", "macros"] }
tracing = { version = "0.1.35" }
# tidy-alphabetical-end

Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_driver_impl/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ use rustc_span::source_map::FileLoader;
use rustc_target::json::ToJson;
use rustc_target::spec::{Target, TargetTuple};
use time::OffsetDateTime;
use time::macros::format_description;
use tracing::trace;

#[allow(unused_macros)]
Expand Down Expand Up @@ -1356,8 +1357,7 @@ fn ice_path_with_config(config: Option<&UnstableOptions>) -> &'static Option<Pat
let file_now = now
.format(
// Don't use a standard datetime format because Windows doesn't support `:` in paths
&time::format_description::parse("[year]-[month]-[day]T[hour]_[minute]_[second]")
.unwrap(),
&format_description!("[year]-[month]-[day]T[hour]_[minute]_[second]"),
)
.unwrap_or_default();
let pid = std::process::id();
Expand Down
18 changes: 6 additions & 12 deletions compiler/rustc_incremental/src/persist/fs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -585,23 +585,17 @@ fn extract_timestamp_from_session_dir(directory_name: &str) -> Result<SystemTime

fn timestamp_to_string(timestamp: SystemTime) -> BaseNString {
let duration = timestamp.duration_since(UNIX_EPOCH).unwrap();
let micros = duration.as_secs() * 1_000_000 + (duration.subsec_nanos() as u64) / 1000;
let micros: u64 = duration.as_micros().try_into().unwrap();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've made conversions explicitly fallible here and other places.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we using u64 here? to_base_fixed_len seems to support u128 too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

    println!("{}", 1234567_u64.to_base_fixed_len(CASE_INSENSITIVE));
    println!("{}", 1234567_u128.to_base_fixed_len(CASE_INSENSITIVE));

will give

000000000qglj
000000000000000000000qglj

so this will change timestamp part of filename making it longer, idk if that needed?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm... i guess if we never expect it to overflow u64 in practice then it's fine. 2^64 microseconds seems to be 584 thousand years...

micros.to_base_fixed_len(CASE_INSENSITIVE)
}

fn string_to_timestamp(s: &str) -> Result<SystemTime, &'static str> {
let micros_since_unix_epoch = u64::from_str_radix(s, INT_ENCODE_BASE as u32);

if micros_since_unix_epoch.is_err() {
return Err("timestamp not an int");
}

let micros_since_unix_epoch = micros_since_unix_epoch.unwrap();
let micros_since_unix_epoch = match u64::from_str_radix(s, INT_ENCODE_BASE as u32) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use u128 here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is deserialize part of timestamp_to_string <--> string_to_timestamp, so depends on previous one encoding.

Ok(micros) => micros,
Err(_) => return Err("timestamp not an int"),
};

let duration = Duration::new(
micros_since_unix_epoch / 1_000_000,
1000 * (micros_since_unix_epoch % 1_000_000) as u32,
);
let duration = Duration::from_micros(micros_since_unix_epoch);
Ok(UNIX_EPOCH + duration)
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_query_system/src/dep_graph/graph.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1098,7 +1098,7 @@ impl<D: Deps> CurrentDepGraph<D> {
use std::time::{SystemTime, UNIX_EPOCH};

let duration = SystemTime::now().duration_since(UNIX_EPOCH).unwrap();
let nanos = duration.as_secs() * 1_000_000_000 + duration.subsec_nanos() as u64;
let nanos = duration.as_nanos();
let mut stable_hasher = StableHasher::new();
nanos.hash(&mut stable_hasher);
let anon_id_seed = stable_hasher.finish();
Expand Down
Loading