Redundant comparison & “if” before assignment
Here is the example:
if(value != ageValue) {
ageValue = value;
}
I mean, if we assign the value of a variable to another one, why would we need to check if they have anyway the same value?
That confuses me. Here is the broader context:
private double ageValue;
public double Age {
get {
return ageValue;
}
set {
if(value != ageValue) {
ageValue = value;
}
}
}
c# .net if-statement
New contributor
|
show 13 more comments
Here is the example:
if(value != ageValue) {
ageValue = value;
}
I mean, if we assign the value of a variable to another one, why would we need to check if they have anyway the same value?
That confuses me. Here is the broader context:
private double ageValue;
public double Age {
get {
return ageValue;
}
set {
if(value != ageValue) {
ageValue = value;
}
}
}
c# .net if-statement
New contributor
17
assuming ageValue isn't a property, but is only a variable then there is no point. If ageValue is a property, maybe something happens in the set?
– Alex Anderson
Mar 22 at 12:57
9
So you only set the ageValue when the value is something new. Right here it doesn't make sense to have anif
but in other cases when we do more than just set the value it can save time. For example, in a WPF MVVM application afterageValue = value
we'd most likely callNotifyPropertyChanged
so the GUI knows that a property changed, but we'd only want to do this if our property actually changed
– MindSwipe
Mar 22 at 13:03
5
With regard to your added/edited code: yes, theif
is redundant in this particular code example you have given.
– elgonzo
Mar 22 at 13:08
5
Related: Is it a sensible optimization to check whether a variable holds a specific value before writing that value?
– GSerg
Mar 22 at 16:08
5
@SeM: It's getting upvotes because the most absurdly trivial questions are usually the most popular. That's unfortunately true across all of stackexchange, since for any topic, there are many more beginners than there are experts.
– BlueRaja - Danny Pflughoeft
Mar 22 at 19:45
|
show 13 more comments
Here is the example:
if(value != ageValue) {
ageValue = value;
}
I mean, if we assign the value of a variable to another one, why would we need to check if they have anyway the same value?
That confuses me. Here is the broader context:
private double ageValue;
public double Age {
get {
return ageValue;
}
set {
if(value != ageValue) {
ageValue = value;
}
}
}
c# .net if-statement
New contributor
Here is the example:
if(value != ageValue) {
ageValue = value;
}
I mean, if we assign the value of a variable to another one, why would we need to check if they have anyway the same value?
That confuses me. Here is the broader context:
private double ageValue;
public double Age {
get {
return ageValue;
}
set {
if(value != ageValue) {
ageValue = value;
}
}
}
c# .net if-statement
c# .net if-statement
New contributor
New contributor
edited Mar 22 at 20:30
Solomon Ucko
7752822
7752822
New contributor
asked Mar 22 at 12:55
TheOrlexxTheOrlexx
18827
18827
New contributor
New contributor
17
assuming ageValue isn't a property, but is only a variable then there is no point. If ageValue is a property, maybe something happens in the set?
– Alex Anderson
Mar 22 at 12:57
9
So you only set the ageValue when the value is something new. Right here it doesn't make sense to have anif
but in other cases when we do more than just set the value it can save time. For example, in a WPF MVVM application afterageValue = value
we'd most likely callNotifyPropertyChanged
so the GUI knows that a property changed, but we'd only want to do this if our property actually changed
– MindSwipe
Mar 22 at 13:03
5
With regard to your added/edited code: yes, theif
is redundant in this particular code example you have given.
– elgonzo
Mar 22 at 13:08
5
Related: Is it a sensible optimization to check whether a variable holds a specific value before writing that value?
– GSerg
Mar 22 at 16:08
5
@SeM: It's getting upvotes because the most absurdly trivial questions are usually the most popular. That's unfortunately true across all of stackexchange, since for any topic, there are many more beginners than there are experts.
– BlueRaja - Danny Pflughoeft
Mar 22 at 19:45
|
show 13 more comments
17
assuming ageValue isn't a property, but is only a variable then there is no point. If ageValue is a property, maybe something happens in the set?
– Alex Anderson
Mar 22 at 12:57
9
So you only set the ageValue when the value is something new. Right here it doesn't make sense to have anif
but in other cases when we do more than just set the value it can save time. For example, in a WPF MVVM application afterageValue = value
we'd most likely callNotifyPropertyChanged
so the GUI knows that a property changed, but we'd only want to do this if our property actually changed
– MindSwipe
Mar 22 at 13:03
5
With regard to your added/edited code: yes, theif
is redundant in this particular code example you have given.
– elgonzo
Mar 22 at 13:08
5
Related: Is it a sensible optimization to check whether a variable holds a specific value before writing that value?
– GSerg
Mar 22 at 16:08
5
@SeM: It's getting upvotes because the most absurdly trivial questions are usually the most popular. That's unfortunately true across all of stackexchange, since for any topic, there are many more beginners than there are experts.
– BlueRaja - Danny Pflughoeft
Mar 22 at 19:45
17
17
assuming ageValue isn't a property, but is only a variable then there is no point. If ageValue is a property, maybe something happens in the set?
– Alex Anderson
Mar 22 at 12:57
assuming ageValue isn't a property, but is only a variable then there is no point. If ageValue is a property, maybe something happens in the set?
– Alex Anderson
Mar 22 at 12:57
9
9
So you only set the ageValue when the value is something new. Right here it doesn't make sense to have an
if
but in other cases when we do more than just set the value it can save time. For example, in a WPF MVVM application after ageValue = value
we'd most likely call NotifyPropertyChanged
so the GUI knows that a property changed, but we'd only want to do this if our property actually changed– MindSwipe
Mar 22 at 13:03
So you only set the ageValue when the value is something new. Right here it doesn't make sense to have an
if
but in other cases when we do more than just set the value it can save time. For example, in a WPF MVVM application after ageValue = value
we'd most likely call NotifyPropertyChanged
so the GUI knows that a property changed, but we'd only want to do this if our property actually changed– MindSwipe
Mar 22 at 13:03
5
5
With regard to your added/edited code: yes, the
if
is redundant in this particular code example you have given.– elgonzo
Mar 22 at 13:08
With regard to your added/edited code: yes, the
if
is redundant in this particular code example you have given.– elgonzo
Mar 22 at 13:08
5
5
Related: Is it a sensible optimization to check whether a variable holds a specific value before writing that value?
– GSerg
Mar 22 at 16:08
Related: Is it a sensible optimization to check whether a variable holds a specific value before writing that value?
– GSerg
Mar 22 at 16:08
5
5
@SeM: It's getting upvotes because the most absurdly trivial questions are usually the most popular. That's unfortunately true across all of stackexchange, since for any topic, there are many more beginners than there are experts.
– BlueRaja - Danny Pflughoeft
Mar 22 at 19:45
@SeM: It's getting upvotes because the most absurdly trivial questions are usually the most popular. That's unfortunately true across all of stackexchange, since for any topic, there are many more beginners than there are experts.
– BlueRaja - Danny Pflughoeft
Mar 22 at 19:45
|
show 13 more comments
6 Answers
6
active
oldest
votes
Here is a code sample when the check is quite useful:
public class MyClass {
...
int ageValue = 0;
public int AgeValue {
get {
return ageValue
}
protected set {
... // value validation here
// your code starts
if (value != ageValue) {
ageValue = value;
}
// your code ends
else
return; // do nothing since value == ageValue
// ageValue has been changed
// Time (or / and memory) consuming process
SaveToRDBMS();
InvalidateCache();
...
}
}
...
More natural implementation, however, is to check in the very beginning in order to avoid unnecessary computation.
protected set {
if (ageValue == value)
return;
... // value validation here
ageValue = value;
// ageValue has been changed
// Time (or / and memory) consuming process
SaveToRDBMS();
InvalidateCache();
...
}
5
True, however I don't believe that a "time (or / and memory) consuming process" is a good thing to do in the setter. After all, this is why we can't useasync/await
on properties. See the answer of this question..
– Guilherme
Mar 22 at 18:40
4
Personally I'd have the check beif (value == ageValue) return;
– mowwwalker
Mar 22 at 19:04
1
@Guilherme true, however, this construction is very useful also in case you're implementing INotifyPropertyChanged or related data binding handlers. You generally don't want to trigger the change tracking code if nothing changes. However small the overhead, it's still unnecessary, and, for high-frequency code, quickly adds up.
– Tom Lint
2 days ago
add a comment |
In a winforms control we had set the BackgroundColor to a specific color:
myControl.BackgroundColor = Color.White
Under specific circumstances this could happen in a tight loop and lead to a frozen UI. After some performance analysis we found that this call was the reason for the frozen UI and so we simply changed it to:
if (myControl.BackgroundColor != Color.White)
myControl.BackgroundColor = Color.White
And the performance of our tool was back on track (and then we eliminated the reason of the tight loop).
So this check is not always redundant. Especially if the target is a property which does more within the setter then simply applying the value to a backing store.
add a comment |
The if
is, on inspection, not redundant. It depends on the remaining implementation. Note that in C#, !=
can be overloaded, which means that evaluation can have side effects. Futhermore, the checked variables could be implemented as properties, which also can have side effects on evaluation.
26
And if you find != has been overloaded, or someone has implemented properties with grossly unintuitive side effects, immediately draw and quarter the responsible developer.
– BobbyA
Mar 22 at 13:10
@BobbyA: The side effects don't have to be unintuitive, just expensive
– Ben Voigt
2 days ago
add a comment |
This question has gained quite some comments but so far all answers try to reframe the question to address issues with operator overloading or side effects of the setter.
If the setter is used by multiple threads it can really make a difference. The check before set pattern can (you should measure) be useful if you are iterating over the same data with multiple threads which alter the data. The text book name for this phenomena is called false sharing. If you read the data and did verify that it already matches the target value you can omit the write.
If you omit the write the CPU does not need to flush the cache line (a 64 byte block on Intel CPUs) to ensure that other cores see the changed value. If the other core was about to read some other data from that 64 byte block then you just have slowed down your core and increased cross core traffic to synchronize memory contents between CPU caches.
The following sample application shows this effect which also contains the check before write condition:
if (tmp1 != checkValue) // set only if not equal to checkvalue
{
values[i] = checkValue;
}
Here is the full code:
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using System.Threading.Tasks;
class Program
{
static void Main(string args)
{
const int N = 500_000_000;
int values = new int[N]; // 2 GB
for (int nThreads = 1; nThreads < Environment.ProcessorCount; nThreads++)
{
SetArray(values, checkValue: 1, nTimes: 10, nThreads: nThreads);
SetArray(values, checkValue: 2, nTimes: 10, nThreads: nThreads);
SetArrayNoCheck(values, checkValue: 2, nTimes: 10, nThreads: nThreads);
}
}
private static void SetArray(int values, int checkValue, int nTimes, int nThreads)
{
List<double> ms = new List<double>();
for (int k = 0; k < nTimes; k++) // set array values to 1
{
for (int i = 0; i < values.Length; i++)
{
values[i] = 1;
}
var sw = Stopwatch.StartNew();
Action acc = () =>
{
int tmp1 = 0;
for (int i = 0; i < values.Length; i++)
{
tmp1 = values[i];
if (tmp1 != checkValue) // set only if not equal to checkvalue
{
values[i] = checkValue;
}
}
};
Parallel.Invoke(Enumerable.Repeat(acc, nThreads).ToArray()); // Let this run on 3 cores
sw.Stop();
ms.Add(sw.Elapsed.TotalMilliseconds);
// Console.WriteLine($"Set {values.Length * 4 / (1_000_000_000.0f):F1} GB of Memory in {sw.Elapsed.TotalMilliseconds:F0} ms. Initial Value 1. Set Value {checkValue}");
}
string descr = checkValue == 1 ? "Conditional Not Set" : "Conditional Set";
Console.WriteLine($"{descr}, {ms.Average():F0}, ms, nThreads, {nThreads}");
}
private static void SetArrayNoCheck(int values, int checkValue, int nTimes, int nThreads)
{
List<double> ms = new List<double>();
for (int k = 0; k < nTimes; k++) // set array values to 1
{
for (int i = 0; i < values.Length; i++)
{
values[i] = 1;
}
var sw = Stopwatch.StartNew();
Action acc = () =>
{
for (int i = 0; i < values.Length; i++)
{
values[i] = checkValue;
}
};
Parallel.Invoke(Enumerable.Repeat(acc, nThreads).ToArray()); // Let this run on 3 cores
sw.Stop();
ms.Add(sw.Elapsed.TotalMilliseconds);
//Console.WriteLine($"Unconditional Set {values.Length * 4 / (1_000_000_000.0f):F1} GB of Memory in {sw.Elapsed.TotalMilliseconds:F0} ms. Initial Value 1. Set Value {checkValue}");
}
Console.WriteLine($"Unconditional Set, {ms.Average():F0}, ms, nThreads, {nThreads}");
}
}
If you let that run you get values like:
// Value not set
Set 2.0 GB of Memory in 439 ms. Initial Value 1. Set Value 1
Set 2.0 GB of Memory in 420 ms. Initial Value 1. Set Value 1
Set 2.0 GB of Memory in 429 ms. Initial Value 1. Set Value 1
Set 2.0 GB of Memory in 393 ms. Initial Value 1. Set Value 1
Set 2.0 GB of Memory in 404 ms. Initial Value 1. Set Value 1
Set 2.0 GB of Memory in 395 ms. Initial Value 1. Set Value 1
Set 2.0 GB of Memory in 419 ms. Initial Value 1. Set Value 1
Set 2.0 GB of Memory in 421 ms. Initial Value 1. Set Value 1
Set 2.0 GB of Memory in 442 ms. Initial Value 1. Set Value 1
Set 2.0 GB of Memory in 422 ms. Initial Value 1. Set Value 1
// Value written
Set 2.0 GB of Memory in 519 ms. Initial Value 1. Set Value 2
Set 2.0 GB of Memory in 582 ms. Initial Value 1. Set Value 2
Set 2.0 GB of Memory in 543 ms. Initial Value 1. Set Value 2
Set 2.0 GB of Memory in 484 ms. Initial Value 1. Set Value 2
Set 2.0 GB of Memory in 523 ms. Initial Value 1. Set Value 2
Set 2.0 GB of Memory in 540 ms. Initial Value 1. Set Value 2
Set 2.0 GB of Memory in 552 ms. Initial Value 1. Set Value 2
Set 2.0 GB of Memory in 527 ms. Initial Value 1. Set Value 2
Set 2.0 GB of Memory in 535 ms. Initial Value 1. Set Value 2
Set 2.0 GB of Memory in 581 ms. Initial Value 1. Set Value 2
That results in a 22% faster performance which can be significant in high performance number crunching scenarios.
To answer the question as it was written:
You can remove the if statement if access to the memory is only single threaded. If multiple threads are working on the same or nearby data false sharing can happen which can cost you up to ca. 20% of memory access performance.
Update 1
I have ran more tests and created a chart to show the cross core chit chat. This shows a simple set (Unconditional Set) as it was noted by commenter Frank Hopkins. Conditional Not Set contains the if which never sets the value. And last but not least Conditional Set will set the value in the if condition.
2
If the setter is used by multiple threads without any synchronization that essentially means the produced data are inconsitent, and at this point the processing speed isn't really that important. C# doesn't require writes to non-volatile fields in one thread to be visible in other threads until a volatile field referenced, or alock
taken/released, or a thread created/terminated.
– Joker_vD
Mar 22 at 15:39
2
That is a simplified example. Suppose you have a struct with two fields int F1, F2. And you update them in two threads where one updates only F1 and the other F2. There is no overlap in the written data but your performance will be down due to false sharing. That is meant with false sharing. You have a false data dependency due to how caches of the CPU work.
– Alois Kraus
Mar 22 at 16:05
2
@Joker_vD: If different threads were writing meaningfully-different data, that would be true. False sharing can occur, however, in cases where all threads that would modify something would store the same value or equivalent values. For example, if an object has a field to say whether it has ever done something, setting the flag totrue
when it already is may degrade the performance of other threads trying to read the flag.
– supercat
Mar 22 at 17:39
1
hmm, the example code actually compares doing three operations on each element (retrieve value, compare value, increment counter) against doing four operations on each element (retrieve, compare, set, increment). Naturally the latter has to be slower (if each operation takes roughly the same time, by about 25%... ). A more indicative comparison would be code that has the check against code that doesn't have the check at all.
– Frank Hopkins
2 days ago
@FrankHopkins: I have created a chart and altered the code a bit to show also an unconditional set. Funny side note: Running this on .NET Core 3.0 Beta shows much worse perf. Looks like this is still a debug build or at least with many checks enabled.
– Alois Kraus
2 days ago
|
show 1 more comment
I've actually coded stuff like this a few times, for different reasons. They're kinda hard to explain, so bear with me.
The main thing is that you don't set a new reference if the value at the reference is logically equal to the prior reference's value. In comments above, users have criticized the obnoxiousness of this scenario – and it is obnoxious to have to deal with – but still essentially necessary in cases.
I'd try to split up use cases like this:
The value is an abstract data type, where you may have different constructed instances representing the same logical value.
- This happens a lot in math programs, e.g. Mathematica, where you can't use primitive numerics, allowing you to end up with different objects meant to represent the same.
The reference of
value
is useful to a caching logic.
- This can also pop up when using abstract numerics. For example, if you expect other parts of the program to have cached data about a reference, then you don't want to replace it with a logically equivalent reference, as it'll invalidate the caches used elsewhere.
You're using a reactive evaluator, where setting a new value may forces a chain-reaction of updates.
- Exactly how and why this matters varies depending on the context.
The big conceptual point is that, in some cases, you can have the same logical value stored at different references, but you want to try to minimize the number of degenerate references for two big reasons:
Having the same logical value stored multiple times hogs more memory.
A lot of the run-time can use reference-checking as a shortcut, e.g. through caching, which can be more efficient if you avoid allowing redundant references to the same logical value to propagate.
For another random example, .NET's garbage collector is "generational", meaning that it puts more effort into checking if a value can be collected when it's newer. So, the garbage collector can experience gains if you preferentially retain the older reference, as it's in a more privileged generation, allowing the newer reference to get garbage collected sooner.
Another use case, again with abstract data types, is where you might have lazily-evaluated properties attached to them. For example, say you have an abstract class Number
that has properties like .IsRational
, .IsEven
, etc.. Then, you might not calculate those immediately, but rather generate them on-demand, caching the results. In a scenario like this, you may tend to prefer to retain older Number
's of the same logical value as they may have more stuff attached to them, whereas a new value
may have less information associated with it, even if it's logically ==
.
It's kinda hard to think of how to sum up the various reasons why this can make sense in some cases, but it's basically an optimization that can make sense if you have a reason to use it. If you don't have any reason to use it, then probably best to not worry about it until some motivation arises.
add a comment |
Yes, this if
is useless. You check if the value are the same (and set it if not).
When the !=
-operator is not overloaded, then is this:
private double ageValue;
public double Age
{
get { return ageValue; }
set
{
if (value != ageValue)
{
ageValue = value;
}
}
}
same to
private double ageValue;
public double Age
{
get { return ageValue; }
set { ageValue = value; }
}
Is it useless if the they are properties with getters and setters that perform other operations?
– Amy
Mar 22 at 13:05
Its the setter-method (look at the question). There can not be any other actions. The only thing (but happend not often out there) is a overloaded!=
-operator.
– user6537157
Mar 22 at 13:13
Yes, the question was expanded with additional context.
– Amy
Mar 22 at 13:14
Why is the only answer that addresses OP's actual question rather than a "what if" so heavily downvoted?
– helrich
Mar 22 at 13:14
Because before revision 4 of the question, this answer was wrong. Codor's answer was the only one that was correct.
– Amy
Mar 22 at 13:15
|
show 3 more comments
Your Answer
StackExchange.ifUsing("editor", function () {
StackExchange.using("externalEditor", function () {
StackExchange.using("snippets", function () {
StackExchange.snippets.init();
});
});
}, "code-snippets");
StackExchange.ready(function() {
var channelOptions = {
tags: "".split(" "),
id: "1"
};
initTagRenderer("".split(" "), "".split(" "), channelOptions);
StackExchange.using("externalEditor", function() {
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled) {
StackExchange.using("snippets", function() {
createEditor();
});
}
else {
createEditor();
}
});
function createEditor() {
StackExchange.prepareEditor({
heartbeatType: 'answer',
autoActivateHeartbeat: false,
convertImagesToLinks: true,
noModals: true,
showLowRepImageUploadWarning: true,
reputationToPostImages: 10,
bindNavPrevention: true,
postfix: "",
imageUploader: {
brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
allowUrls: true
},
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
});
}
});
TheOrlexx is a new contributor. Be nice, and check out our Code of Conduct.
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fstackoverflow.com%2fquestions%2f55300081%2fredundant-comparison-if-before-assignment%23new-answer', 'question_page');
}
);
Post as a guest
Required, but never shown
6 Answers
6
active
oldest
votes
6 Answers
6
active
oldest
votes
active
oldest
votes
active
oldest
votes
Here is a code sample when the check is quite useful:
public class MyClass {
...
int ageValue = 0;
public int AgeValue {
get {
return ageValue
}
protected set {
... // value validation here
// your code starts
if (value != ageValue) {
ageValue = value;
}
// your code ends
else
return; // do nothing since value == ageValue
// ageValue has been changed
// Time (or / and memory) consuming process
SaveToRDBMS();
InvalidateCache();
...
}
}
...
More natural implementation, however, is to check in the very beginning in order to avoid unnecessary computation.
protected set {
if (ageValue == value)
return;
... // value validation here
ageValue = value;
// ageValue has been changed
// Time (or / and memory) consuming process
SaveToRDBMS();
InvalidateCache();
...
}
5
True, however I don't believe that a "time (or / and memory) consuming process" is a good thing to do in the setter. After all, this is why we can't useasync/await
on properties. See the answer of this question..
– Guilherme
Mar 22 at 18:40
4
Personally I'd have the check beif (value == ageValue) return;
– mowwwalker
Mar 22 at 19:04
1
@Guilherme true, however, this construction is very useful also in case you're implementing INotifyPropertyChanged or related data binding handlers. You generally don't want to trigger the change tracking code if nothing changes. However small the overhead, it's still unnecessary, and, for high-frequency code, quickly adds up.
– Tom Lint
2 days ago
add a comment |
Here is a code sample when the check is quite useful:
public class MyClass {
...
int ageValue = 0;
public int AgeValue {
get {
return ageValue
}
protected set {
... // value validation here
// your code starts
if (value != ageValue) {
ageValue = value;
}
// your code ends
else
return; // do nothing since value == ageValue
// ageValue has been changed
// Time (or / and memory) consuming process
SaveToRDBMS();
InvalidateCache();
...
}
}
...
More natural implementation, however, is to check in the very beginning in order to avoid unnecessary computation.
protected set {
if (ageValue == value)
return;
... // value validation here
ageValue = value;
// ageValue has been changed
// Time (or / and memory) consuming process
SaveToRDBMS();
InvalidateCache();
...
}
5
True, however I don't believe that a "time (or / and memory) consuming process" is a good thing to do in the setter. After all, this is why we can't useasync/await
on properties. See the answer of this question..
– Guilherme
Mar 22 at 18:40
4
Personally I'd have the check beif (value == ageValue) return;
– mowwwalker
Mar 22 at 19:04
1
@Guilherme true, however, this construction is very useful also in case you're implementing INotifyPropertyChanged or related data binding handlers. You generally don't want to trigger the change tracking code if nothing changes. However small the overhead, it's still unnecessary, and, for high-frequency code, quickly adds up.
– Tom Lint
2 days ago
add a comment |
Here is a code sample when the check is quite useful:
public class MyClass {
...
int ageValue = 0;
public int AgeValue {
get {
return ageValue
}
protected set {
... // value validation here
// your code starts
if (value != ageValue) {
ageValue = value;
}
// your code ends
else
return; // do nothing since value == ageValue
// ageValue has been changed
// Time (or / and memory) consuming process
SaveToRDBMS();
InvalidateCache();
...
}
}
...
More natural implementation, however, is to check in the very beginning in order to avoid unnecessary computation.
protected set {
if (ageValue == value)
return;
... // value validation here
ageValue = value;
// ageValue has been changed
// Time (or / and memory) consuming process
SaveToRDBMS();
InvalidateCache();
...
}
Here is a code sample when the check is quite useful:
public class MyClass {
...
int ageValue = 0;
public int AgeValue {
get {
return ageValue
}
protected set {
... // value validation here
// your code starts
if (value != ageValue) {
ageValue = value;
}
// your code ends
else
return; // do nothing since value == ageValue
// ageValue has been changed
// Time (or / and memory) consuming process
SaveToRDBMS();
InvalidateCache();
...
}
}
...
More natural implementation, however, is to check in the very beginning in order to avoid unnecessary computation.
protected set {
if (ageValue == value)
return;
... // value validation here
ageValue = value;
// ageValue has been changed
// Time (or / and memory) consuming process
SaveToRDBMS();
InvalidateCache();
...
}
edited Mar 22 at 21:28
Keaton Thomas
111
111
answered Mar 22 at 13:08
Dmitry BychenkoDmitry Bychenko
111k1098140
111k1098140
5
True, however I don't believe that a "time (or / and memory) consuming process" is a good thing to do in the setter. After all, this is why we can't useasync/await
on properties. See the answer of this question..
– Guilherme
Mar 22 at 18:40
4
Personally I'd have the check beif (value == ageValue) return;
– mowwwalker
Mar 22 at 19:04
1
@Guilherme true, however, this construction is very useful also in case you're implementing INotifyPropertyChanged or related data binding handlers. You generally don't want to trigger the change tracking code if nothing changes. However small the overhead, it's still unnecessary, and, for high-frequency code, quickly adds up.
– Tom Lint
2 days ago
add a comment |
5
True, however I don't believe that a "time (or / and memory) consuming process" is a good thing to do in the setter. After all, this is why we can't useasync/await
on properties. See the answer of this question..
– Guilherme
Mar 22 at 18:40
4
Personally I'd have the check beif (value == ageValue) return;
– mowwwalker
Mar 22 at 19:04
1
@Guilherme true, however, this construction is very useful also in case you're implementing INotifyPropertyChanged or related data binding handlers. You generally don't want to trigger the change tracking code if nothing changes. However small the overhead, it's still unnecessary, and, for high-frequency code, quickly adds up.
– Tom Lint
2 days ago
5
5
True, however I don't believe that a "time (or / and memory) consuming process" is a good thing to do in the setter. After all, this is why we can't use
async/await
on properties. See the answer of this question..– Guilherme
Mar 22 at 18:40
True, however I don't believe that a "time (or / and memory) consuming process" is a good thing to do in the setter. After all, this is why we can't use
async/await
on properties. See the answer of this question..– Guilherme
Mar 22 at 18:40
4
4
Personally I'd have the check be
if (value == ageValue) return;
– mowwwalker
Mar 22 at 19:04
Personally I'd have the check be
if (value == ageValue) return;
– mowwwalker
Mar 22 at 19:04
1
1
@Guilherme true, however, this construction is very useful also in case you're implementing INotifyPropertyChanged or related data binding handlers. You generally don't want to trigger the change tracking code if nothing changes. However small the overhead, it's still unnecessary, and, for high-frequency code, quickly adds up.
– Tom Lint
2 days ago
@Guilherme true, however, this construction is very useful also in case you're implementing INotifyPropertyChanged or related data binding handlers. You generally don't want to trigger the change tracking code if nothing changes. However small the overhead, it's still unnecessary, and, for high-frequency code, quickly adds up.
– Tom Lint
2 days ago
add a comment |
In a winforms control we had set the BackgroundColor to a specific color:
myControl.BackgroundColor = Color.White
Under specific circumstances this could happen in a tight loop and lead to a frozen UI. After some performance analysis we found that this call was the reason for the frozen UI and so we simply changed it to:
if (myControl.BackgroundColor != Color.White)
myControl.BackgroundColor = Color.White
And the performance of our tool was back on track (and then we eliminated the reason of the tight loop).
So this check is not always redundant. Especially if the target is a property which does more within the setter then simply applying the value to a backing store.
add a comment |
In a winforms control we had set the BackgroundColor to a specific color:
myControl.BackgroundColor = Color.White
Under specific circumstances this could happen in a tight loop and lead to a frozen UI. After some performance analysis we found that this call was the reason for the frozen UI and so we simply changed it to:
if (myControl.BackgroundColor != Color.White)
myControl.BackgroundColor = Color.White
And the performance of our tool was back on track (and then we eliminated the reason of the tight loop).
So this check is not always redundant. Especially if the target is a property which does more within the setter then simply applying the value to a backing store.
add a comment |
In a winforms control we had set the BackgroundColor to a specific color:
myControl.BackgroundColor = Color.White
Under specific circumstances this could happen in a tight loop and lead to a frozen UI. After some performance analysis we found that this call was the reason for the frozen UI and so we simply changed it to:
if (myControl.BackgroundColor != Color.White)
myControl.BackgroundColor = Color.White
And the performance of our tool was back on track (and then we eliminated the reason of the tight loop).
So this check is not always redundant. Especially if the target is a property which does more within the setter then simply applying the value to a backing store.
In a winforms control we had set the BackgroundColor to a specific color:
myControl.BackgroundColor = Color.White
Under specific circumstances this could happen in a tight loop and lead to a frozen UI. After some performance analysis we found that this call was the reason for the frozen UI and so we simply changed it to:
if (myControl.BackgroundColor != Color.White)
myControl.BackgroundColor = Color.White
And the performance of our tool was back on track (and then we eliminated the reason of the tight loop).
So this check is not always redundant. Especially if the target is a property which does more within the setter then simply applying the value to a backing store.
edited Mar 22 at 18:31
Chris Hayes
8,12832241
8,12832241
answered Mar 22 at 13:06
OliverOliver
33.2k772118
33.2k772118
add a comment |
add a comment |
The if
is, on inspection, not redundant. It depends on the remaining implementation. Note that in C#, !=
can be overloaded, which means that evaluation can have side effects. Futhermore, the checked variables could be implemented as properties, which also can have side effects on evaluation.
26
And if you find != has been overloaded, or someone has implemented properties with grossly unintuitive side effects, immediately draw and quarter the responsible developer.
– BobbyA
Mar 22 at 13:10
@BobbyA: The side effects don't have to be unintuitive, just expensive
– Ben Voigt
2 days ago
add a comment |
The if
is, on inspection, not redundant. It depends on the remaining implementation. Note that in C#, !=
can be overloaded, which means that evaluation can have side effects. Futhermore, the checked variables could be implemented as properties, which also can have side effects on evaluation.
26
And if you find != has been overloaded, or someone has implemented properties with grossly unintuitive side effects, immediately draw and quarter the responsible developer.
– BobbyA
Mar 22 at 13:10
@BobbyA: The side effects don't have to be unintuitive, just expensive
– Ben Voigt
2 days ago
add a comment |
The if
is, on inspection, not redundant. It depends on the remaining implementation. Note that in C#, !=
can be overloaded, which means that evaluation can have side effects. Futhermore, the checked variables could be implemented as properties, which also can have side effects on evaluation.
The if
is, on inspection, not redundant. It depends on the remaining implementation. Note that in C#, !=
can be overloaded, which means that evaluation can have side effects. Futhermore, the checked variables could be implemented as properties, which also can have side effects on evaluation.
answered Mar 22 at 13:00
CodorCodor
15.6k82648
15.6k82648
26
And if you find != has been overloaded, or someone has implemented properties with grossly unintuitive side effects, immediately draw and quarter the responsible developer.
– BobbyA
Mar 22 at 13:10
@BobbyA: The side effects don't have to be unintuitive, just expensive
– Ben Voigt
2 days ago
add a comment |
26
And if you find != has been overloaded, or someone has implemented properties with grossly unintuitive side effects, immediately draw and quarter the responsible developer.
– BobbyA
Mar 22 at 13:10
@BobbyA: The side effects don't have to be unintuitive, just expensive
– Ben Voigt
2 days ago
26
26
And if you find != has been overloaded, or someone has implemented properties with grossly unintuitive side effects, immediately draw and quarter the responsible developer.
– BobbyA
Mar 22 at 13:10
And if you find != has been overloaded, or someone has implemented properties with grossly unintuitive side effects, immediately draw and quarter the responsible developer.
– BobbyA
Mar 22 at 13:10
@BobbyA: The side effects don't have to be unintuitive, just expensive
– Ben Voigt
2 days ago
@BobbyA: The side effects don't have to be unintuitive, just expensive
– Ben Voigt
2 days ago
add a comment |
This question has gained quite some comments but so far all answers try to reframe the question to address issues with operator overloading or side effects of the setter.
If the setter is used by multiple threads it can really make a difference. The check before set pattern can (you should measure) be useful if you are iterating over the same data with multiple threads which alter the data. The text book name for this phenomena is called false sharing. If you read the data and did verify that it already matches the target value you can omit the write.
If you omit the write the CPU does not need to flush the cache line (a 64 byte block on Intel CPUs) to ensure that other cores see the changed value. If the other core was about to read some other data from that 64 byte block then you just have slowed down your core and increased cross core traffic to synchronize memory contents between CPU caches.
The following sample application shows this effect which also contains the check before write condition:
if (tmp1 != checkValue) // set only if not equal to checkvalue
{
values[i] = checkValue;
}
Here is the full code:
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using System.Threading.Tasks;
class Program
{
static void Main(string args)
{
const int N = 500_000_000;
int values = new int[N]; // 2 GB
for (int nThreads = 1; nThreads < Environment.ProcessorCount; nThreads++)
{
SetArray(values, checkValue: 1, nTimes: 10, nThreads: nThreads);
SetArray(values, checkValue: 2, nTimes: 10, nThreads: nThreads);
SetArrayNoCheck(values, checkValue: 2, nTimes: 10, nThreads: nThreads);
}
}
private static void SetArray(int values, int checkValue, int nTimes, int nThreads)
{
List<double> ms = new List<double>();
for (int k = 0; k < nTimes; k++) // set array values to 1
{
for (int i = 0; i < values.Length; i++)
{
values[i] = 1;
}
var sw = Stopwatch.StartNew();
Action acc = () =>
{
int tmp1 = 0;
for (int i = 0; i < values.Length; i++)
{
tmp1 = values[i];
if (tmp1 != checkValue) // set only if not equal to checkvalue
{
values[i] = checkValue;
}
}
};
Parallel.Invoke(Enumerable.Repeat(acc, nThreads).ToArray()); // Let this run on 3 cores
sw.Stop();
ms.Add(sw.Elapsed.TotalMilliseconds);
// Console.WriteLine($"Set {values.Length * 4 / (1_000_000_000.0f):F1} GB of Memory in {sw.Elapsed.TotalMilliseconds:F0} ms. Initial Value 1. Set Value {checkValue}");
}
string descr = checkValue == 1 ? "Conditional Not Set" : "Conditional Set";
Console.WriteLine($"{descr}, {ms.Average():F0}, ms, nThreads, {nThreads}");
}
private static void SetArrayNoCheck(int values, int checkValue, int nTimes, int nThreads)
{
List<double> ms = new List<double>();
for (int k = 0; k < nTimes; k++) // set array values to 1
{
for (int i = 0; i < values.Length; i++)
{
values[i] = 1;
}
var sw = Stopwatch.StartNew();
Action acc = () =>
{
for (int i = 0; i < values.Length; i++)
{
values[i] = checkValue;
}
};
Parallel.Invoke(Enumerable.Repeat(acc, nThreads).ToArray()); // Let this run on 3 cores
sw.Stop();
ms.Add(sw.Elapsed.TotalMilliseconds);
//Console.WriteLine($"Unconditional Set {values.Length * 4 / (1_000_000_000.0f):F1} GB of Memory in {sw.Elapsed.TotalMilliseconds:F0} ms. Initial Value 1. Set Value {checkValue}");
}
Console.WriteLine($"Unconditional Set, {ms.Average():F0}, ms, nThreads, {nThreads}");
}
}
If you let that run you get values like:
// Value not set
Set 2.0 GB of Memory in 439 ms. Initial Value 1. Set Value 1
Set 2.0 GB of Memory in 420 ms. Initial Value 1. Set Value 1
Set 2.0 GB of Memory in 429 ms. Initial Value 1. Set Value 1
Set 2.0 GB of Memory in 393 ms. Initial Value 1. Set Value 1
Set 2.0 GB of Memory in 404 ms. Initial Value 1. Set Value 1
Set 2.0 GB of Memory in 395 ms. Initial Value 1. Set Value 1
Set 2.0 GB of Memory in 419 ms. Initial Value 1. Set Value 1
Set 2.0 GB of Memory in 421 ms. Initial Value 1. Set Value 1
Set 2.0 GB of Memory in 442 ms. Initial Value 1. Set Value 1
Set 2.0 GB of Memory in 422 ms. Initial Value 1. Set Value 1
// Value written
Set 2.0 GB of Memory in 519 ms. Initial Value 1. Set Value 2
Set 2.0 GB of Memory in 582 ms. Initial Value 1. Set Value 2
Set 2.0 GB of Memory in 543 ms. Initial Value 1. Set Value 2
Set 2.0 GB of Memory in 484 ms. Initial Value 1. Set Value 2
Set 2.0 GB of Memory in 523 ms. Initial Value 1. Set Value 2
Set 2.0 GB of Memory in 540 ms. Initial Value 1. Set Value 2
Set 2.0 GB of Memory in 552 ms. Initial Value 1. Set Value 2
Set 2.0 GB of Memory in 527 ms. Initial Value 1. Set Value 2
Set 2.0 GB of Memory in 535 ms. Initial Value 1. Set Value 2
Set 2.0 GB of Memory in 581 ms. Initial Value 1. Set Value 2
That results in a 22% faster performance which can be significant in high performance number crunching scenarios.
To answer the question as it was written:
You can remove the if statement if access to the memory is only single threaded. If multiple threads are working on the same or nearby data false sharing can happen which can cost you up to ca. 20% of memory access performance.
Update 1
I have ran more tests and created a chart to show the cross core chit chat. This shows a simple set (Unconditional Set) as it was noted by commenter Frank Hopkins. Conditional Not Set contains the if which never sets the value. And last but not least Conditional Set will set the value in the if condition.
2
If the setter is used by multiple threads without any synchronization that essentially means the produced data are inconsitent, and at this point the processing speed isn't really that important. C# doesn't require writes to non-volatile fields in one thread to be visible in other threads until a volatile field referenced, or alock
taken/released, or a thread created/terminated.
– Joker_vD
Mar 22 at 15:39
2
That is a simplified example. Suppose you have a struct with two fields int F1, F2. And you update them in two threads where one updates only F1 and the other F2. There is no overlap in the written data but your performance will be down due to false sharing. That is meant with false sharing. You have a false data dependency due to how caches of the CPU work.
– Alois Kraus
Mar 22 at 16:05
2
@Joker_vD: If different threads were writing meaningfully-different data, that would be true. False sharing can occur, however, in cases where all threads that would modify something would store the same value or equivalent values. For example, if an object has a field to say whether it has ever done something, setting the flag totrue
when it already is may degrade the performance of other threads trying to read the flag.
– supercat
Mar 22 at 17:39
1
hmm, the example code actually compares doing three operations on each element (retrieve value, compare value, increment counter) against doing four operations on each element (retrieve, compare, set, increment). Naturally the latter has to be slower (if each operation takes roughly the same time, by about 25%... ). A more indicative comparison would be code that has the check against code that doesn't have the check at all.
– Frank Hopkins
2 days ago
@FrankHopkins: I have created a chart and altered the code a bit to show also an unconditional set. Funny side note: Running this on .NET Core 3.0 Beta shows much worse perf. Looks like this is still a debug build or at least with many checks enabled.
– Alois Kraus
2 days ago
|
show 1 more comment
This question has gained quite some comments but so far all answers try to reframe the question to address issues with operator overloading or side effects of the setter.
If the setter is used by multiple threads it can really make a difference. The check before set pattern can (you should measure) be useful if you are iterating over the same data with multiple threads which alter the data. The text book name for this phenomena is called false sharing. If you read the data and did verify that it already matches the target value you can omit the write.
If you omit the write the CPU does not need to flush the cache line (a 64 byte block on Intel CPUs) to ensure that other cores see the changed value. If the other core was about to read some other data from that 64 byte block then you just have slowed down your core and increased cross core traffic to synchronize memory contents between CPU caches.
The following sample application shows this effect which also contains the check before write condition:
if (tmp1 != checkValue) // set only if not equal to checkvalue
{
values[i] = checkValue;
}
Here is the full code:
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using System.Threading.Tasks;
class Program
{
static void Main(string args)
{
const int N = 500_000_000;
int values = new int[N]; // 2 GB
for (int nThreads = 1; nThreads < Environment.ProcessorCount; nThreads++)
{
SetArray(values, checkValue: 1, nTimes: 10, nThreads: nThreads);
SetArray(values, checkValue: 2, nTimes: 10, nThreads: nThreads);
SetArrayNoCheck(values, checkValue: 2, nTimes: 10, nThreads: nThreads);
}
}
private static void SetArray(int values, int checkValue, int nTimes, int nThreads)
{
List<double> ms = new List<double>();
for (int k = 0; k < nTimes; k++) // set array values to 1
{
for (int i = 0; i < values.Length; i++)
{
values[i] = 1;
}
var sw = Stopwatch.StartNew();
Action acc = () =>
{
int tmp1 = 0;
for (int i = 0; i < values.Length; i++)
{
tmp1 = values[i];
if (tmp1 != checkValue) // set only if not equal to checkvalue
{
values[i] = checkValue;
}
}
};
Parallel.Invoke(Enumerable.Repeat(acc, nThreads).ToArray()); // Let this run on 3 cores
sw.Stop();
ms.Add(sw.Elapsed.TotalMilliseconds);
// Console.WriteLine($"Set {values.Length * 4 / (1_000_000_000.0f):F1} GB of Memory in {sw.Elapsed.TotalMilliseconds:F0} ms. Initial Value 1. Set Value {checkValue}");
}
string descr = checkValue == 1 ? "Conditional Not Set" : "Conditional Set";
Console.WriteLine($"{descr}, {ms.Average():F0}, ms, nThreads, {nThreads}");
}
private static void SetArrayNoCheck(int values, int checkValue, int nTimes, int nThreads)
{
List<double> ms = new List<double>();
for (int k = 0; k < nTimes; k++) // set array values to 1
{
for (int i = 0; i < values.Length; i++)
{
values[i] = 1;
}
var sw = Stopwatch.StartNew();
Action acc = () =>
{
for (int i = 0; i < values.Length; i++)
{
values[i] = checkValue;
}
};
Parallel.Invoke(Enumerable.Repeat(acc, nThreads).ToArray()); // Let this run on 3 cores
sw.Stop();
ms.Add(sw.Elapsed.TotalMilliseconds);
//Console.WriteLine($"Unconditional Set {values.Length * 4 / (1_000_000_000.0f):F1} GB of Memory in {sw.Elapsed.TotalMilliseconds:F0} ms. Initial Value 1. Set Value {checkValue}");
}
Console.WriteLine($"Unconditional Set, {ms.Average():F0}, ms, nThreads, {nThreads}");
}
}
If you let that run you get values like:
// Value not set
Set 2.0 GB of Memory in 439 ms. Initial Value 1. Set Value 1
Set 2.0 GB of Memory in 420 ms. Initial Value 1. Set Value 1
Set 2.0 GB of Memory in 429 ms. Initial Value 1. Set Value 1
Set 2.0 GB of Memory in 393 ms. Initial Value 1. Set Value 1
Set 2.0 GB of Memory in 404 ms. Initial Value 1. Set Value 1
Set 2.0 GB of Memory in 395 ms. Initial Value 1. Set Value 1
Set 2.0 GB of Memory in 419 ms. Initial Value 1. Set Value 1
Set 2.0 GB of Memory in 421 ms. Initial Value 1. Set Value 1
Set 2.0 GB of Memory in 442 ms. Initial Value 1. Set Value 1
Set 2.0 GB of Memory in 422 ms. Initial Value 1. Set Value 1
// Value written
Set 2.0 GB of Memory in 519 ms. Initial Value 1. Set Value 2
Set 2.0 GB of Memory in 582 ms. Initial Value 1. Set Value 2
Set 2.0 GB of Memory in 543 ms. Initial Value 1. Set Value 2
Set 2.0 GB of Memory in 484 ms. Initial Value 1. Set Value 2
Set 2.0 GB of Memory in 523 ms. Initial Value 1. Set Value 2
Set 2.0 GB of Memory in 540 ms. Initial Value 1. Set Value 2
Set 2.0 GB of Memory in 552 ms. Initial Value 1. Set Value 2
Set 2.0 GB of Memory in 527 ms. Initial Value 1. Set Value 2
Set 2.0 GB of Memory in 535 ms. Initial Value 1. Set Value 2
Set 2.0 GB of Memory in 581 ms. Initial Value 1. Set Value 2
That results in a 22% faster performance which can be significant in high performance number crunching scenarios.
To answer the question as it was written:
You can remove the if statement if access to the memory is only single threaded. If multiple threads are working on the same or nearby data false sharing can happen which can cost you up to ca. 20% of memory access performance.
Update 1
I have ran more tests and created a chart to show the cross core chit chat. This shows a simple set (Unconditional Set) as it was noted by commenter Frank Hopkins. Conditional Not Set contains the if which never sets the value. And last but not least Conditional Set will set the value in the if condition.
2
If the setter is used by multiple threads without any synchronization that essentially means the produced data are inconsitent, and at this point the processing speed isn't really that important. C# doesn't require writes to non-volatile fields in one thread to be visible in other threads until a volatile field referenced, or alock
taken/released, or a thread created/terminated.
– Joker_vD
Mar 22 at 15:39
2
That is a simplified example. Suppose you have a struct with two fields int F1, F2. And you update them in two threads where one updates only F1 and the other F2. There is no overlap in the written data but your performance will be down due to false sharing. That is meant with false sharing. You have a false data dependency due to how caches of the CPU work.
– Alois Kraus
Mar 22 at 16:05
2
@Joker_vD: If different threads were writing meaningfully-different data, that would be true. False sharing can occur, however, in cases where all threads that would modify something would store the same value or equivalent values. For example, if an object has a field to say whether it has ever done something, setting the flag totrue
when it already is may degrade the performance of other threads trying to read the flag.
– supercat
Mar 22 at 17:39
1
hmm, the example code actually compares doing three operations on each element (retrieve value, compare value, increment counter) against doing four operations on each element (retrieve, compare, set, increment). Naturally the latter has to be slower (if each operation takes roughly the same time, by about 25%... ). A more indicative comparison would be code that has the check against code that doesn't have the check at all.
– Frank Hopkins
2 days ago
@FrankHopkins: I have created a chart and altered the code a bit to show also an unconditional set. Funny side note: Running this on .NET Core 3.0 Beta shows much worse perf. Looks like this is still a debug build or at least with many checks enabled.
– Alois Kraus
2 days ago
|
show 1 more comment
This question has gained quite some comments but so far all answers try to reframe the question to address issues with operator overloading or side effects of the setter.
If the setter is used by multiple threads it can really make a difference. The check before set pattern can (you should measure) be useful if you are iterating over the same data with multiple threads which alter the data. The text book name for this phenomena is called false sharing. If you read the data and did verify that it already matches the target value you can omit the write.
If you omit the write the CPU does not need to flush the cache line (a 64 byte block on Intel CPUs) to ensure that other cores see the changed value. If the other core was about to read some other data from that 64 byte block then you just have slowed down your core and increased cross core traffic to synchronize memory contents between CPU caches.
The following sample application shows this effect which also contains the check before write condition:
if (tmp1 != checkValue) // set only if not equal to checkvalue
{
values[i] = checkValue;
}
Here is the full code:
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using System.Threading.Tasks;
class Program
{
static void Main(string args)
{
const int N = 500_000_000;
int values = new int[N]; // 2 GB
for (int nThreads = 1; nThreads < Environment.ProcessorCount; nThreads++)
{
SetArray(values, checkValue: 1, nTimes: 10, nThreads: nThreads);
SetArray(values, checkValue: 2, nTimes: 10, nThreads: nThreads);
SetArrayNoCheck(values, checkValue: 2, nTimes: 10, nThreads: nThreads);
}
}
private static void SetArray(int values, int checkValue, int nTimes, int nThreads)
{
List<double> ms = new List<double>();
for (int k = 0; k < nTimes; k++) // set array values to 1
{
for (int i = 0; i < values.Length; i++)
{
values[i] = 1;
}
var sw = Stopwatch.StartNew();
Action acc = () =>
{
int tmp1 = 0;
for (int i = 0; i < values.Length; i++)
{
tmp1 = values[i];
if (tmp1 != checkValue) // set only if not equal to checkvalue
{
values[i] = checkValue;
}
}
};
Parallel.Invoke(Enumerable.Repeat(acc, nThreads).ToArray()); // Let this run on 3 cores
sw.Stop();
ms.Add(sw.Elapsed.TotalMilliseconds);
// Console.WriteLine($"Set {values.Length * 4 / (1_000_000_000.0f):F1} GB of Memory in {sw.Elapsed.TotalMilliseconds:F0} ms. Initial Value 1. Set Value {checkValue}");
}
string descr = checkValue == 1 ? "Conditional Not Set" : "Conditional Set";
Console.WriteLine($"{descr}, {ms.Average():F0}, ms, nThreads, {nThreads}");
}
private static void SetArrayNoCheck(int values, int checkValue, int nTimes, int nThreads)
{
List<double> ms = new List<double>();
for (int k = 0; k < nTimes; k++) // set array values to 1
{
for (int i = 0; i < values.Length; i++)
{
values[i] = 1;
}
var sw = Stopwatch.StartNew();
Action acc = () =>
{
for (int i = 0; i < values.Length; i++)
{
values[i] = checkValue;
}
};
Parallel.Invoke(Enumerable.Repeat(acc, nThreads).ToArray()); // Let this run on 3 cores
sw.Stop();
ms.Add(sw.Elapsed.TotalMilliseconds);
//Console.WriteLine($"Unconditional Set {values.Length * 4 / (1_000_000_000.0f):F1} GB of Memory in {sw.Elapsed.TotalMilliseconds:F0} ms. Initial Value 1. Set Value {checkValue}");
}
Console.WriteLine($"Unconditional Set, {ms.Average():F0}, ms, nThreads, {nThreads}");
}
}
If you let that run you get values like:
// Value not set
Set 2.0 GB of Memory in 439 ms. Initial Value 1. Set Value 1
Set 2.0 GB of Memory in 420 ms. Initial Value 1. Set Value 1
Set 2.0 GB of Memory in 429 ms. Initial Value 1. Set Value 1
Set 2.0 GB of Memory in 393 ms. Initial Value 1. Set Value 1
Set 2.0 GB of Memory in 404 ms. Initial Value 1. Set Value 1
Set 2.0 GB of Memory in 395 ms. Initial Value 1. Set Value 1
Set 2.0 GB of Memory in 419 ms. Initial Value 1. Set Value 1
Set 2.0 GB of Memory in 421 ms. Initial Value 1. Set Value 1
Set 2.0 GB of Memory in 442 ms. Initial Value 1. Set Value 1
Set 2.0 GB of Memory in 422 ms. Initial Value 1. Set Value 1
// Value written
Set 2.0 GB of Memory in 519 ms. Initial Value 1. Set Value 2
Set 2.0 GB of Memory in 582 ms. Initial Value 1. Set Value 2
Set 2.0 GB of Memory in 543 ms. Initial Value 1. Set Value 2
Set 2.0 GB of Memory in 484 ms. Initial Value 1. Set Value 2
Set 2.0 GB of Memory in 523 ms. Initial Value 1. Set Value 2
Set 2.0 GB of Memory in 540 ms. Initial Value 1. Set Value 2
Set 2.0 GB of Memory in 552 ms. Initial Value 1. Set Value 2
Set 2.0 GB of Memory in 527 ms. Initial Value 1. Set Value 2
Set 2.0 GB of Memory in 535 ms. Initial Value 1. Set Value 2
Set 2.0 GB of Memory in 581 ms. Initial Value 1. Set Value 2
That results in a 22% faster performance which can be significant in high performance number crunching scenarios.
To answer the question as it was written:
You can remove the if statement if access to the memory is only single threaded. If multiple threads are working on the same or nearby data false sharing can happen which can cost you up to ca. 20% of memory access performance.
Update 1
I have ran more tests and created a chart to show the cross core chit chat. This shows a simple set (Unconditional Set) as it was noted by commenter Frank Hopkins. Conditional Not Set contains the if which never sets the value. And last but not least Conditional Set will set the value in the if condition.
This question has gained quite some comments but so far all answers try to reframe the question to address issues with operator overloading or side effects of the setter.
If the setter is used by multiple threads it can really make a difference. The check before set pattern can (you should measure) be useful if you are iterating over the same data with multiple threads which alter the data. The text book name for this phenomena is called false sharing. If you read the data and did verify that it already matches the target value you can omit the write.
If you omit the write the CPU does not need to flush the cache line (a 64 byte block on Intel CPUs) to ensure that other cores see the changed value. If the other core was about to read some other data from that 64 byte block then you just have slowed down your core and increased cross core traffic to synchronize memory contents between CPU caches.
The following sample application shows this effect which also contains the check before write condition:
if (tmp1 != checkValue) // set only if not equal to checkvalue
{
values[i] = checkValue;
}
Here is the full code:
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using System.Threading.Tasks;
class Program
{
static void Main(string args)
{
const int N = 500_000_000;
int values = new int[N]; // 2 GB
for (int nThreads = 1; nThreads < Environment.ProcessorCount; nThreads++)
{
SetArray(values, checkValue: 1, nTimes: 10, nThreads: nThreads);
SetArray(values, checkValue: 2, nTimes: 10, nThreads: nThreads);
SetArrayNoCheck(values, checkValue: 2, nTimes: 10, nThreads: nThreads);
}
}
private static void SetArray(int values, int checkValue, int nTimes, int nThreads)
{
List<double> ms = new List<double>();
for (int k = 0; k < nTimes; k++) // set array values to 1
{
for (int i = 0; i < values.Length; i++)
{
values[i] = 1;
}
var sw = Stopwatch.StartNew();
Action acc = () =>
{
int tmp1 = 0;
for (int i = 0; i < values.Length; i++)
{
tmp1 = values[i];
if (tmp1 != checkValue) // set only if not equal to checkvalue
{
values[i] = checkValue;
}
}
};
Parallel.Invoke(Enumerable.Repeat(acc, nThreads).ToArray()); // Let this run on 3 cores
sw.Stop();
ms.Add(sw.Elapsed.TotalMilliseconds);
// Console.WriteLine($"Set {values.Length * 4 / (1_000_000_000.0f):F1} GB of Memory in {sw.Elapsed.TotalMilliseconds:F0} ms. Initial Value 1. Set Value {checkValue}");
}
string descr = checkValue == 1 ? "Conditional Not Set" : "Conditional Set";
Console.WriteLine($"{descr}, {ms.Average():F0}, ms, nThreads, {nThreads}");
}
private static void SetArrayNoCheck(int values, int checkValue, int nTimes, int nThreads)
{
List<double> ms = new List<double>();
for (int k = 0; k < nTimes; k++) // set array values to 1
{
for (int i = 0; i < values.Length; i++)
{
values[i] = 1;
}
var sw = Stopwatch.StartNew();
Action acc = () =>
{
for (int i = 0; i < values.Length; i++)
{
values[i] = checkValue;
}
};
Parallel.Invoke(Enumerable.Repeat(acc, nThreads).ToArray()); // Let this run on 3 cores
sw.Stop();
ms.Add(sw.Elapsed.TotalMilliseconds);
//Console.WriteLine($"Unconditional Set {values.Length * 4 / (1_000_000_000.0f):F1} GB of Memory in {sw.Elapsed.TotalMilliseconds:F0} ms. Initial Value 1. Set Value {checkValue}");
}
Console.WriteLine($"Unconditional Set, {ms.Average():F0}, ms, nThreads, {nThreads}");
}
}
If you let that run you get values like:
// Value not set
Set 2.0 GB of Memory in 439 ms. Initial Value 1. Set Value 1
Set 2.0 GB of Memory in 420 ms. Initial Value 1. Set Value 1
Set 2.0 GB of Memory in 429 ms. Initial Value 1. Set Value 1
Set 2.0 GB of Memory in 393 ms. Initial Value 1. Set Value 1
Set 2.0 GB of Memory in 404 ms. Initial Value 1. Set Value 1
Set 2.0 GB of Memory in 395 ms. Initial Value 1. Set Value 1
Set 2.0 GB of Memory in 419 ms. Initial Value 1. Set Value 1
Set 2.0 GB of Memory in 421 ms. Initial Value 1. Set Value 1
Set 2.0 GB of Memory in 442 ms. Initial Value 1. Set Value 1
Set 2.0 GB of Memory in 422 ms. Initial Value 1. Set Value 1
// Value written
Set 2.0 GB of Memory in 519 ms. Initial Value 1. Set Value 2
Set 2.0 GB of Memory in 582 ms. Initial Value 1. Set Value 2
Set 2.0 GB of Memory in 543 ms. Initial Value 1. Set Value 2
Set 2.0 GB of Memory in 484 ms. Initial Value 1. Set Value 2
Set 2.0 GB of Memory in 523 ms. Initial Value 1. Set Value 2
Set 2.0 GB of Memory in 540 ms. Initial Value 1. Set Value 2
Set 2.0 GB of Memory in 552 ms. Initial Value 1. Set Value 2
Set 2.0 GB of Memory in 527 ms. Initial Value 1. Set Value 2
Set 2.0 GB of Memory in 535 ms. Initial Value 1. Set Value 2
Set 2.0 GB of Memory in 581 ms. Initial Value 1. Set Value 2
That results in a 22% faster performance which can be significant in high performance number crunching scenarios.
To answer the question as it was written:
You can remove the if statement if access to the memory is only single threaded. If multiple threads are working on the same or nearby data false sharing can happen which can cost you up to ca. 20% of memory access performance.
Update 1
I have ran more tests and created a chart to show the cross core chit chat. This shows a simple set (Unconditional Set) as it was noted by commenter Frank Hopkins. Conditional Not Set contains the if which never sets the value. And last but not least Conditional Set will set the value in the if condition.
edited 14 hours ago
answered Mar 22 at 15:03
Alois KrausAlois Kraus
10.3k2453
10.3k2453
2
If the setter is used by multiple threads without any synchronization that essentially means the produced data are inconsitent, and at this point the processing speed isn't really that important. C# doesn't require writes to non-volatile fields in one thread to be visible in other threads until a volatile field referenced, or alock
taken/released, or a thread created/terminated.
– Joker_vD
Mar 22 at 15:39
2
That is a simplified example. Suppose you have a struct with two fields int F1, F2. And you update them in two threads where one updates only F1 and the other F2. There is no overlap in the written data but your performance will be down due to false sharing. That is meant with false sharing. You have a false data dependency due to how caches of the CPU work.
– Alois Kraus
Mar 22 at 16:05
2
@Joker_vD: If different threads were writing meaningfully-different data, that would be true. False sharing can occur, however, in cases where all threads that would modify something would store the same value or equivalent values. For example, if an object has a field to say whether it has ever done something, setting the flag totrue
when it already is may degrade the performance of other threads trying to read the flag.
– supercat
Mar 22 at 17:39
1
hmm, the example code actually compares doing three operations on each element (retrieve value, compare value, increment counter) against doing four operations on each element (retrieve, compare, set, increment). Naturally the latter has to be slower (if each operation takes roughly the same time, by about 25%... ). A more indicative comparison would be code that has the check against code that doesn't have the check at all.
– Frank Hopkins
2 days ago
@FrankHopkins: I have created a chart and altered the code a bit to show also an unconditional set. Funny side note: Running this on .NET Core 3.0 Beta shows much worse perf. Looks like this is still a debug build or at least with many checks enabled.
– Alois Kraus
2 days ago
|
show 1 more comment
2
If the setter is used by multiple threads without any synchronization that essentially means the produced data are inconsitent, and at this point the processing speed isn't really that important. C# doesn't require writes to non-volatile fields in one thread to be visible in other threads until a volatile field referenced, or alock
taken/released, or a thread created/terminated.
– Joker_vD
Mar 22 at 15:39
2
That is a simplified example. Suppose you have a struct with two fields int F1, F2. And you update them in two threads where one updates only F1 and the other F2. There is no overlap in the written data but your performance will be down due to false sharing. That is meant with false sharing. You have a false data dependency due to how caches of the CPU work.
– Alois Kraus
Mar 22 at 16:05
2
@Joker_vD: If different threads were writing meaningfully-different data, that would be true. False sharing can occur, however, in cases where all threads that would modify something would store the same value or equivalent values. For example, if an object has a field to say whether it has ever done something, setting the flag totrue
when it already is may degrade the performance of other threads trying to read the flag.
– supercat
Mar 22 at 17:39
1
hmm, the example code actually compares doing three operations on each element (retrieve value, compare value, increment counter) against doing four operations on each element (retrieve, compare, set, increment). Naturally the latter has to be slower (if each operation takes roughly the same time, by about 25%... ). A more indicative comparison would be code that has the check against code that doesn't have the check at all.
– Frank Hopkins
2 days ago
@FrankHopkins: I have created a chart and altered the code a bit to show also an unconditional set. Funny side note: Running this on .NET Core 3.0 Beta shows much worse perf. Looks like this is still a debug build or at least with many checks enabled.
– Alois Kraus
2 days ago
2
2
If the setter is used by multiple threads without any synchronization that essentially means the produced data are inconsitent, and at this point the processing speed isn't really that important. C# doesn't require writes to non-volatile fields in one thread to be visible in other threads until a volatile field referenced, or a
lock
taken/released, or a thread created/terminated.– Joker_vD
Mar 22 at 15:39
If the setter is used by multiple threads without any synchronization that essentially means the produced data are inconsitent, and at this point the processing speed isn't really that important. C# doesn't require writes to non-volatile fields in one thread to be visible in other threads until a volatile field referenced, or a
lock
taken/released, or a thread created/terminated.– Joker_vD
Mar 22 at 15:39
2
2
That is a simplified example. Suppose you have a struct with two fields int F1, F2. And you update them in two threads where one updates only F1 and the other F2. There is no overlap in the written data but your performance will be down due to false sharing. That is meant with false sharing. You have a false data dependency due to how caches of the CPU work.
– Alois Kraus
Mar 22 at 16:05
That is a simplified example. Suppose you have a struct with two fields int F1, F2. And you update them in two threads where one updates only F1 and the other F2. There is no overlap in the written data but your performance will be down due to false sharing. That is meant with false sharing. You have a false data dependency due to how caches of the CPU work.
– Alois Kraus
Mar 22 at 16:05
2
2
@Joker_vD: If different threads were writing meaningfully-different data, that would be true. False sharing can occur, however, in cases where all threads that would modify something would store the same value or equivalent values. For example, if an object has a field to say whether it has ever done something, setting the flag to
true
when it already is may degrade the performance of other threads trying to read the flag.– supercat
Mar 22 at 17:39
@Joker_vD: If different threads were writing meaningfully-different data, that would be true. False sharing can occur, however, in cases where all threads that would modify something would store the same value or equivalent values. For example, if an object has a field to say whether it has ever done something, setting the flag to
true
when it already is may degrade the performance of other threads trying to read the flag.– supercat
Mar 22 at 17:39
1
1
hmm, the example code actually compares doing three operations on each element (retrieve value, compare value, increment counter) against doing four operations on each element (retrieve, compare, set, increment). Naturally the latter has to be slower (if each operation takes roughly the same time, by about 25%... ). A more indicative comparison would be code that has the check against code that doesn't have the check at all.
– Frank Hopkins
2 days ago
hmm, the example code actually compares doing three operations on each element (retrieve value, compare value, increment counter) against doing four operations on each element (retrieve, compare, set, increment). Naturally the latter has to be slower (if each operation takes roughly the same time, by about 25%... ). A more indicative comparison would be code that has the check against code that doesn't have the check at all.
– Frank Hopkins
2 days ago
@FrankHopkins: I have created a chart and altered the code a bit to show also an unconditional set. Funny side note: Running this on .NET Core 3.0 Beta shows much worse perf. Looks like this is still a debug build or at least with many checks enabled.
– Alois Kraus
2 days ago
@FrankHopkins: I have created a chart and altered the code a bit to show also an unconditional set. Funny side note: Running this on .NET Core 3.0 Beta shows much worse perf. Looks like this is still a debug build or at least with many checks enabled.
– Alois Kraus
2 days ago
|
show 1 more comment
I've actually coded stuff like this a few times, for different reasons. They're kinda hard to explain, so bear with me.
The main thing is that you don't set a new reference if the value at the reference is logically equal to the prior reference's value. In comments above, users have criticized the obnoxiousness of this scenario – and it is obnoxious to have to deal with – but still essentially necessary in cases.
I'd try to split up use cases like this:
The value is an abstract data type, where you may have different constructed instances representing the same logical value.
- This happens a lot in math programs, e.g. Mathematica, where you can't use primitive numerics, allowing you to end up with different objects meant to represent the same.
The reference of
value
is useful to a caching logic.
- This can also pop up when using abstract numerics. For example, if you expect other parts of the program to have cached data about a reference, then you don't want to replace it with a logically equivalent reference, as it'll invalidate the caches used elsewhere.
You're using a reactive evaluator, where setting a new value may forces a chain-reaction of updates.
- Exactly how and why this matters varies depending on the context.
The big conceptual point is that, in some cases, you can have the same logical value stored at different references, but you want to try to minimize the number of degenerate references for two big reasons:
Having the same logical value stored multiple times hogs more memory.
A lot of the run-time can use reference-checking as a shortcut, e.g. through caching, which can be more efficient if you avoid allowing redundant references to the same logical value to propagate.
For another random example, .NET's garbage collector is "generational", meaning that it puts more effort into checking if a value can be collected when it's newer. So, the garbage collector can experience gains if you preferentially retain the older reference, as it's in a more privileged generation, allowing the newer reference to get garbage collected sooner.
Another use case, again with abstract data types, is where you might have lazily-evaluated properties attached to them. For example, say you have an abstract class Number
that has properties like .IsRational
, .IsEven
, etc.. Then, you might not calculate those immediately, but rather generate them on-demand, caching the results. In a scenario like this, you may tend to prefer to retain older Number
's of the same logical value as they may have more stuff attached to them, whereas a new value
may have less information associated with it, even if it's logically ==
.
It's kinda hard to think of how to sum up the various reasons why this can make sense in some cases, but it's basically an optimization that can make sense if you have a reason to use it. If you don't have any reason to use it, then probably best to not worry about it until some motivation arises.
add a comment |
I've actually coded stuff like this a few times, for different reasons. They're kinda hard to explain, so bear with me.
The main thing is that you don't set a new reference if the value at the reference is logically equal to the prior reference's value. In comments above, users have criticized the obnoxiousness of this scenario – and it is obnoxious to have to deal with – but still essentially necessary in cases.
I'd try to split up use cases like this:
The value is an abstract data type, where you may have different constructed instances representing the same logical value.
- This happens a lot in math programs, e.g. Mathematica, where you can't use primitive numerics, allowing you to end up with different objects meant to represent the same.
The reference of
value
is useful to a caching logic.
- This can also pop up when using abstract numerics. For example, if you expect other parts of the program to have cached data about a reference, then you don't want to replace it with a logically equivalent reference, as it'll invalidate the caches used elsewhere.
You're using a reactive evaluator, where setting a new value may forces a chain-reaction of updates.
- Exactly how and why this matters varies depending on the context.
The big conceptual point is that, in some cases, you can have the same logical value stored at different references, but you want to try to minimize the number of degenerate references for two big reasons:
Having the same logical value stored multiple times hogs more memory.
A lot of the run-time can use reference-checking as a shortcut, e.g. through caching, which can be more efficient if you avoid allowing redundant references to the same logical value to propagate.
For another random example, .NET's garbage collector is "generational", meaning that it puts more effort into checking if a value can be collected when it's newer. So, the garbage collector can experience gains if you preferentially retain the older reference, as it's in a more privileged generation, allowing the newer reference to get garbage collected sooner.
Another use case, again with abstract data types, is where you might have lazily-evaluated properties attached to them. For example, say you have an abstract class Number
that has properties like .IsRational
, .IsEven
, etc.. Then, you might not calculate those immediately, but rather generate them on-demand, caching the results. In a scenario like this, you may tend to prefer to retain older Number
's of the same logical value as they may have more stuff attached to them, whereas a new value
may have less information associated with it, even if it's logically ==
.
It's kinda hard to think of how to sum up the various reasons why this can make sense in some cases, but it's basically an optimization that can make sense if you have a reason to use it. If you don't have any reason to use it, then probably best to not worry about it until some motivation arises.
add a comment |
I've actually coded stuff like this a few times, for different reasons. They're kinda hard to explain, so bear with me.
The main thing is that you don't set a new reference if the value at the reference is logically equal to the prior reference's value. In comments above, users have criticized the obnoxiousness of this scenario – and it is obnoxious to have to deal with – but still essentially necessary in cases.
I'd try to split up use cases like this:
The value is an abstract data type, where you may have different constructed instances representing the same logical value.
- This happens a lot in math programs, e.g. Mathematica, where you can't use primitive numerics, allowing you to end up with different objects meant to represent the same.
The reference of
value
is useful to a caching logic.
- This can also pop up when using abstract numerics. For example, if you expect other parts of the program to have cached data about a reference, then you don't want to replace it with a logically equivalent reference, as it'll invalidate the caches used elsewhere.
You're using a reactive evaluator, where setting a new value may forces a chain-reaction of updates.
- Exactly how and why this matters varies depending on the context.
The big conceptual point is that, in some cases, you can have the same logical value stored at different references, but you want to try to minimize the number of degenerate references for two big reasons:
Having the same logical value stored multiple times hogs more memory.
A lot of the run-time can use reference-checking as a shortcut, e.g. through caching, which can be more efficient if you avoid allowing redundant references to the same logical value to propagate.
For another random example, .NET's garbage collector is "generational", meaning that it puts more effort into checking if a value can be collected when it's newer. So, the garbage collector can experience gains if you preferentially retain the older reference, as it's in a more privileged generation, allowing the newer reference to get garbage collected sooner.
Another use case, again with abstract data types, is where you might have lazily-evaluated properties attached to them. For example, say you have an abstract class Number
that has properties like .IsRational
, .IsEven
, etc.. Then, you might not calculate those immediately, but rather generate them on-demand, caching the results. In a scenario like this, you may tend to prefer to retain older Number
's of the same logical value as they may have more stuff attached to them, whereas a new value
may have less information associated with it, even if it's logically ==
.
It's kinda hard to think of how to sum up the various reasons why this can make sense in some cases, but it's basically an optimization that can make sense if you have a reason to use it. If you don't have any reason to use it, then probably best to not worry about it until some motivation arises.
I've actually coded stuff like this a few times, for different reasons. They're kinda hard to explain, so bear with me.
The main thing is that you don't set a new reference if the value at the reference is logically equal to the prior reference's value. In comments above, users have criticized the obnoxiousness of this scenario – and it is obnoxious to have to deal with – but still essentially necessary in cases.
I'd try to split up use cases like this:
The value is an abstract data type, where you may have different constructed instances representing the same logical value.
- This happens a lot in math programs, e.g. Mathematica, where you can't use primitive numerics, allowing you to end up with different objects meant to represent the same.
The reference of
value
is useful to a caching logic.
- This can also pop up when using abstract numerics. For example, if you expect other parts of the program to have cached data about a reference, then you don't want to replace it with a logically equivalent reference, as it'll invalidate the caches used elsewhere.
You're using a reactive evaluator, where setting a new value may forces a chain-reaction of updates.
- Exactly how and why this matters varies depending on the context.
The big conceptual point is that, in some cases, you can have the same logical value stored at different references, but you want to try to minimize the number of degenerate references for two big reasons:
Having the same logical value stored multiple times hogs more memory.
A lot of the run-time can use reference-checking as a shortcut, e.g. through caching, which can be more efficient if you avoid allowing redundant references to the same logical value to propagate.
For another random example, .NET's garbage collector is "generational", meaning that it puts more effort into checking if a value can be collected when it's newer. So, the garbage collector can experience gains if you preferentially retain the older reference, as it's in a more privileged generation, allowing the newer reference to get garbage collected sooner.
Another use case, again with abstract data types, is where you might have lazily-evaluated properties attached to them. For example, say you have an abstract class Number
that has properties like .IsRational
, .IsEven
, etc.. Then, you might not calculate those immediately, but rather generate them on-demand, caching the results. In a scenario like this, you may tend to prefer to retain older Number
's of the same logical value as they may have more stuff attached to them, whereas a new value
may have less information associated with it, even if it's logically ==
.
It's kinda hard to think of how to sum up the various reasons why this can make sense in some cases, but it's basically an optimization that can make sense if you have a reason to use it. If you don't have any reason to use it, then probably best to not worry about it until some motivation arises.
edited 2 days ago
answered 2 days ago
NatNat
61111328
61111328
add a comment |
add a comment |
Yes, this if
is useless. You check if the value are the same (and set it if not).
When the !=
-operator is not overloaded, then is this:
private double ageValue;
public double Age
{
get { return ageValue; }
set
{
if (value != ageValue)
{
ageValue = value;
}
}
}
same to
private double ageValue;
public double Age
{
get { return ageValue; }
set { ageValue = value; }
}
Is it useless if the they are properties with getters and setters that perform other operations?
– Amy
Mar 22 at 13:05
Its the setter-method (look at the question). There can not be any other actions. The only thing (but happend not often out there) is a overloaded!=
-operator.
– user6537157
Mar 22 at 13:13
Yes, the question was expanded with additional context.
– Amy
Mar 22 at 13:14
Why is the only answer that addresses OP's actual question rather than a "what if" so heavily downvoted?
– helrich
Mar 22 at 13:14
Because before revision 4 of the question, this answer was wrong. Codor's answer was the only one that was correct.
– Amy
Mar 22 at 13:15
|
show 3 more comments
Yes, this if
is useless. You check if the value are the same (and set it if not).
When the !=
-operator is not overloaded, then is this:
private double ageValue;
public double Age
{
get { return ageValue; }
set
{
if (value != ageValue)
{
ageValue = value;
}
}
}
same to
private double ageValue;
public double Age
{
get { return ageValue; }
set { ageValue = value; }
}
Is it useless if the they are properties with getters and setters that perform other operations?
– Amy
Mar 22 at 13:05
Its the setter-method (look at the question). There can not be any other actions. The only thing (but happend not often out there) is a overloaded!=
-operator.
– user6537157
Mar 22 at 13:13
Yes, the question was expanded with additional context.
– Amy
Mar 22 at 13:14
Why is the only answer that addresses OP's actual question rather than a "what if" so heavily downvoted?
– helrich
Mar 22 at 13:14
Because before revision 4 of the question, this answer was wrong. Codor's answer was the only one that was correct.
– Amy
Mar 22 at 13:15
|
show 3 more comments
Yes, this if
is useless. You check if the value are the same (and set it if not).
When the !=
-operator is not overloaded, then is this:
private double ageValue;
public double Age
{
get { return ageValue; }
set
{
if (value != ageValue)
{
ageValue = value;
}
}
}
same to
private double ageValue;
public double Age
{
get { return ageValue; }
set { ageValue = value; }
}
Yes, this if
is useless. You check if the value are the same (and set it if not).
When the !=
-operator is not overloaded, then is this:
private double ageValue;
public double Age
{
get { return ageValue; }
set
{
if (value != ageValue)
{
ageValue = value;
}
}
}
same to
private double ageValue;
public double Age
{
get { return ageValue; }
set { ageValue = value; }
}
edited Mar 22 at 13:18
answered Mar 22 at 12:57
user6537157user6537157
363114
363114
Is it useless if the they are properties with getters and setters that perform other operations?
– Amy
Mar 22 at 13:05
Its the setter-method (look at the question). There can not be any other actions. The only thing (but happend not often out there) is a overloaded!=
-operator.
– user6537157
Mar 22 at 13:13
Yes, the question was expanded with additional context.
– Amy
Mar 22 at 13:14
Why is the only answer that addresses OP's actual question rather than a "what if" so heavily downvoted?
– helrich
Mar 22 at 13:14
Because before revision 4 of the question, this answer was wrong. Codor's answer was the only one that was correct.
– Amy
Mar 22 at 13:15
|
show 3 more comments
Is it useless if the they are properties with getters and setters that perform other operations?
– Amy
Mar 22 at 13:05
Its the setter-method (look at the question). There can not be any other actions. The only thing (but happend not often out there) is a overloaded!=
-operator.
– user6537157
Mar 22 at 13:13
Yes, the question was expanded with additional context.
– Amy
Mar 22 at 13:14
Why is the only answer that addresses OP's actual question rather than a "what if" so heavily downvoted?
– helrich
Mar 22 at 13:14
Because before revision 4 of the question, this answer was wrong. Codor's answer was the only one that was correct.
– Amy
Mar 22 at 13:15
Is it useless if the they are properties with getters and setters that perform other operations?
– Amy
Mar 22 at 13:05
Is it useless if the they are properties with getters and setters that perform other operations?
– Amy
Mar 22 at 13:05
Its the setter-method (look at the question). There can not be any other actions. The only thing (but happend not often out there) is a overloaded
!=
-operator.– user6537157
Mar 22 at 13:13
Its the setter-method (look at the question). There can not be any other actions. The only thing (but happend not often out there) is a overloaded
!=
-operator.– user6537157
Mar 22 at 13:13
Yes, the question was expanded with additional context.
– Amy
Mar 22 at 13:14
Yes, the question was expanded with additional context.
– Amy
Mar 22 at 13:14
Why is the only answer that addresses OP's actual question rather than a "what if" so heavily downvoted?
– helrich
Mar 22 at 13:14
Why is the only answer that addresses OP's actual question rather than a "what if" so heavily downvoted?
– helrich
Mar 22 at 13:14
Because before revision 4 of the question, this answer was wrong. Codor's answer was the only one that was correct.
– Amy
Mar 22 at 13:15
Because before revision 4 of the question, this answer was wrong. Codor's answer was the only one that was correct.
– Amy
Mar 22 at 13:15
|
show 3 more comments
TheOrlexx is a new contributor. Be nice, and check out our Code of Conduct.
TheOrlexx is a new contributor. Be nice, and check out our Code of Conduct.
TheOrlexx is a new contributor. Be nice, and check out our Code of Conduct.
TheOrlexx is a new contributor. Be nice, and check out our Code of Conduct.
Thanks for contributing an answer to Stack Overflow!
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
To learn more, see our tips on writing great answers.
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fstackoverflow.com%2fquestions%2f55300081%2fredundant-comparison-if-before-assignment%23new-answer', 'question_page');
}
);
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
17
assuming ageValue isn't a property, but is only a variable then there is no point. If ageValue is a property, maybe something happens in the set?
– Alex Anderson
Mar 22 at 12:57
9
So you only set the ageValue when the value is something new. Right here it doesn't make sense to have an
if
but in other cases when we do more than just set the value it can save time. For example, in a WPF MVVM application afterageValue = value
we'd most likely callNotifyPropertyChanged
so the GUI knows that a property changed, but we'd only want to do this if our property actually changed– MindSwipe
Mar 22 at 13:03
5
With regard to your added/edited code: yes, the
if
is redundant in this particular code example you have given.– elgonzo
Mar 22 at 13:08
5
Related: Is it a sensible optimization to check whether a variable holds a specific value before writing that value?
– GSerg
Mar 22 at 16:08
5
@SeM: It's getting upvotes because the most absurdly trivial questions are usually the most popular. That's unfortunately true across all of stackexchange, since for any topic, there are many more beginners than there are experts.
– BlueRaja - Danny Pflughoeft
Mar 22 at 19:45