From 46176ebbfdb8ec938f8fc37d43dfe6b5d0718604 Mon Sep 17 00:00:00 2001 From: ridiculous_fish Date: Wed, 9 Sep 2020 13:28:19 -0700 Subject: [PATCH 1/2] Optimize Option::clone to Copy when possible Prior to this change, cloning an Option value would branch on whether the Option was Some, even if it would be cheaper to just memcpy the bits. Default the current implementation, and then specialize it for Copy types, to avoid the branch. --- library/core/src/option.rs | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/library/core/src/option.rs b/library/core/src/option.rs index dd7556758be7d..3ab4c4f12982d 100644 --- a/library/core/src/option.rs +++ b/library/core/src/option.rs @@ -1228,7 +1228,7 @@ fn expect_none_failed(msg: &str, value: &dyn fmt::Debug) -> ! { #[stable(feature = "rust1", since = "1.0.0")] impl Clone for Option { #[inline] - fn clone(&self) -> Self { + default fn clone(&self) -> Self { match self { Some(x) => Some(x.clone()), None => None, @@ -1236,7 +1236,7 @@ impl Clone for Option { } #[inline] - fn clone_from(&mut self, source: &Self) { + default fn clone_from(&mut self, source: &Self) { match (self, source) { (Some(to), Some(from)) => to.clone_from(from), (to, from) => *to = from.clone(), @@ -1244,6 +1244,20 @@ impl Clone for Option { } } +#[stable(feature = "rust1", since = "1.0.0")] +impl Clone for Option { + #[inline] + fn clone(&self) -> Self { + *self + } + + #[inline] + fn clone_from(&mut self, source: &Self) { + *self = *source + } +} + + #[stable(feature = "rust1", since = "1.0.0")] impl Default for Option { /// Returns [`None`][Option::None]. From bebfcfdb54e0ba959b3b6b02acfd5f3451dbeaf6 Mon Sep 17 00:00:00 2001 From: ridiculous_fish Date: Wed, 9 Sep 2020 17:38:07 -0700 Subject: [PATCH 2/2] Optimize Option::clone Range is not Copy even if T is Copy, which pessimizes the implementation of Option::clone. Specialize Option::clone for Range so that it can be more efficient. The specialization uses ptr::read to emulate Copy. --- library/core/src/option.rs | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/library/core/src/option.rs b/library/core/src/option.rs index 3ab4c4f12982d..75151faa3e454 100644 --- a/library/core/src/option.rs +++ b/library/core/src/option.rs @@ -134,8 +134,8 @@ use crate::iter::{FromIterator, FusedIterator, TrustedLen}; use crate::pin::Pin; use crate::{ - convert, fmt, hint, mem, - ops::{self, Deref, DerefMut}, + convert, fmt, hint, mem, ptr, + ops::{self, Deref, DerefMut, Range}, }; /// The `Option` type. See [the module level documentation](self) for more. @@ -1257,6 +1257,16 @@ impl Clone for Option { } } +// Range is not Copy even if T is copy (see #27186), +// so provide an efficient implementation. +#[stable(feature = "rust1", since = "1.0.0")] +impl Clone for Option> { + #[inline] + fn clone(&self) -> Self { + // SAFETY: 'self' is not Drop so memcpy is OK. + unsafe { ptr::read(self as *const Self) } + } +} #[stable(feature = "rust1", since = "1.0.0")] impl Default for Option {