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.

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

Optimize clippy trivially_copy_pass_by_ref for 64-bit. #909

Merged
merged 1 commit into from
May 9, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ While some features like the clipboard, menus or file dialogs are not yet availa
- Global `Application` associated functions are instance methods instead, e.g. `Application::global().quit()` instead of the old `Application::quit()`. ([#763] by [@xStrom])
- Timer events will only be delivered to the widgets that requested them. ([#831] by [@sjoshid])
- `Event::Wheel` now contains a `MouseEvent` structure. ([#895] by [@teddemunnik])
- `AppDelegate::command` now receives a `Target` instead of a `&Target`. ([#909] by [@xStrom])

### Deprecated

Expand Down Expand Up @@ -153,6 +154,7 @@ While some features like the clipboard, menus or file dialogs are not yet availa
[#897]: https://github.com/xi-editor/druid/pull/897
[#898]: https://github.com/xi-editor/druid/pull/898
[#900]: https://github.com/xi-editor/druid/pull/900
[#909]: https://github.com/xi-editor/druid/pull/909

## [0.5.0] - 2020-04-01

Expand Down
5 changes: 5 additions & 0 deletions clippy.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# The default clippy value for this is 8 bytes, which is chosen to improve performance on 32-bit.
# Given that druid is being designed for the future and already even mobile phones have 64-bit CPUs,
Copy link
Collaborator

Choose a reason for hiding this comment

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

and already even mobile phones have 64-bit CPUs

Is it correct to place already there?

and even mobile phones already have 64-bit CPUs

feels better to me.

Copy link
Member Author

Choose a reason for hiding this comment

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

Both are fine and you could even put it at the end. Check out this Cambridge dictionary page about it.

# it makes sense to optimize for 64-bit and accept the performance hits on 32-bit.
# 16 bytes is the number of bytes that fits into two 64-bit CPU registers.
trivial-copy-size-limit = 16
4 changes: 2 additions & 2 deletions druid-shell/src/platform/windows/dialog.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,15 +59,15 @@ unsafe fn make_wstrs(spec: &FileSpec) -> (Vec<u16>, Vec<u16>) {
let exts = spec
.extensions
.iter()
.map(normalize_extension)
.map(|s| normalize_extension(*s))
.collect::<Vec<_>>();
let name = format!("{} ({})", spec.name, exts.as_slice().join("; ")).to_wide();
let extensions = exts.as_slice().join(";").to_wide();
(name, extensions)
}

/// add preceding *., trimming preceding *. that might've been included by the user.
fn normalize_extension(ext: &&str) -> String {
fn normalize_extension(ext: &str) -> String {
format!("*.{}", ext.trim_start_matches('*').trim_start_matches('.'))
}

Expand Down
2 changes: 1 addition & 1 deletion druid/examples/blocking_function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ impl AppDelegate<AppState> for Delegate {
fn command(
&mut self,
_ctx: &mut DelegateCtx,
_target: &Target,
_target: Target,
cmd: &Command,
data: &mut AppState,
_env: &Env,
Expand Down
8 changes: 4 additions & 4 deletions druid/examples/multiwin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ impl AppDelegate<State> for Delegate {
fn command(
&mut self,
ctx: &mut DelegateCtx,
target: &Target,
target: Target,
cmd: &Command,
data: &mut State,
_env: &Env,
Expand All @@ -184,7 +184,7 @@ impl AppDelegate<State> for Delegate {
data.selected = *cmd.get_object().unwrap();
let menu = make_menu::<State>(data);
let cmd = Command::new(druid::commands::SET_MENU, menu);
ctx.submit_command(cmd, *id);
ctx.submit_command(cmd, id);
false
}
// wouldn't it be nice if a menu (like a button) could just mutate state
Expand All @@ -193,14 +193,14 @@ impl AppDelegate<State> for Delegate {
data.menu_count += 1;
let menu = make_menu::<State>(data);
let cmd = Command::new(druid::commands::SET_MENU, menu);
ctx.submit_command(cmd, *id);
ctx.submit_command(cmd, id);
false
}
(Target::Window(id), &MENU_DECREMENT_ACTION) => {
data.menu_count = data.menu_count.saturating_sub(1);
let menu = make_menu::<State>(data);
let cmd = Command::new(druid::commands::SET_MENU, menu);
ctx.submit_command(cmd, *id);
ctx.submit_command(cmd, id);
false
}
(_, &MENU_SWITCH_GLOW_ACTION) => {
Expand Down
2 changes: 1 addition & 1 deletion druid/src/app_delegate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ pub trait AppDelegate<T: Data> {
fn command(
&mut self,
ctx: &mut DelegateCtx,
target: &Target,
target: Target,
cmd: &Command,
data: &mut T,
env: &Env,
Expand Down
4 changes: 2 additions & 2 deletions druid/src/win_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ impl<T: Data> Inner<T> {
}
}

fn delegate_cmd(&mut self, target: &Target, cmd: &Command) -> bool {
fn delegate_cmd(&mut self, target: Target, cmd: &Command) -> bool {
self.with_delegate(|del, data, env, ctx| del.command(ctx, target, cmd, data, env))
.unwrap_or(true)
}
Expand Down Expand Up @@ -306,7 +306,7 @@ impl<T: Data> Inner<T> {
}

fn dispatch_cmd(&mut self, target: Target, cmd: Command) {
if !self.delegate_cmd(&target, &cmd) {
if !self.delegate_cmd(target, &cmd) {
return;
}

Expand Down