#Optimize my Code
rancid mantle
Can someone see if there can be done any optimisations about this code?,
I am not sure if the approach I used is correct since open ai chatbot is telling me that using a switch statement, but Im unsure if that s correct
https://paste.ofcode.org/C7UE4PpzL6Pct4b5YLE5jd
copper copper
Area can be made a struct if it only contains structs.
Same remark for Vector3d if it's not a struct already
Could squeeze out performance by not using delegates and replacing them with a switch expression instead
Area a = something switch {
case1 => new Area(...),
case2 => new Area(...),
...
};
yes I will do that if that is true, even tho it would be great if you could go into detail on why the switch statement is faster
copper copper
It does not invoke delegates (does not execute methods).
Switch expressions are syntax sugar that will compile into a bunch of if statements
Delegates sometimes need to capture their context so they can refer to the class the method is in, resulting into more allocations
rancid mantle
so would using a switch be faster than
Area returnOctantArea(int octant){
if(octant==0){
return FUnciton0();
}
if(octant==1){
return FUnciton1();
}
...
}
copper copper
No, it would be the exact same roughly, switch is converted to a bunch of ifs by the compiler
rancid mantle
another question I have, if I got this correct that functions even if non static are stored not in each struct neither by value or by refference?
copper copper
Yeah methods belong to the class / struct itself, not an instance of it
rancid mantle
so there is no need for them to be static anymore?
copper copper
I would keep them static, it's more performant that way.
Instance methods need to be "referenced" by the instance in what's called a Method Table, which essentially maps methods to indices in an array (so instances can get the methods)
rancid mantle
yes the difference tho would be that when the method isn t static, it would not require parameters, so is static still faster? another option which im considdering is to write the functions them self into the switch statement, so I save on one function call or sth
copper copper
I would just get rid of all the methods altogether and create the Area instances into the switch directly
That will squeeze out every drop of performance it can
rancid mantle
Thanks for your advice, my code is much shorter now and I learned some new stuff about functions
static Area FindOctantAndCreateQuadrant(Vector3d point,Vector3d bottom, Vector3d topDiagonal)
{
Vector3d newSize = (bottom - topDiagonal) / 2;
Vector3d Originalcenter = (bottom + topDiagonal) / 2;
int octantX = point.x < Originalcenter.x ? 0 : 1;
int octantY = point.y < Originalcenter.x ? 0 : 2;
int octantZ = point.z < Originalcenter.x ? 0 : 4;
int octant= octantX + octantY + octantZ;
switch (octant)
{
case 0:
return new Area(bottom, newSize, 0);
case 1:
return new Area(bottom + newSize.xnn, newSize, 1);
case 2:
return new Area(bottom + newSize.nyn, newSize, 2);
case 3:
return new Area(bottom + newSize.xyn, newSize, 3);
case 4:
return new Area(bottom + newSize.nnz, newSize, 4);
case 5:
return new Area(bottom + newSize.xnz, newSize, 5);
case 6:
return new Area(bottom + newSize.nyz, newSize, 6);
case 7:
return new Area(bottom + newSize.xyz, newSize, 7);
}
return new Area();
}
Anything you see I could still improve?
Yeah seems like the only thing that changes in all of these switch cases is the + newSize.whatever.
You can offload that calculation into the switch statement, and have just one Area creation:
// switch...
return new Area(bottom + calculatedSwitchValue, newSize, octant)
The octant is also now used for the third argument as it maps with the switch cases 1:1
Also look into switch expressions which are different but would be more suited for your case here!
(they are as performant as a switch statement, they also compile to if statements)
rancid mantle
//Big thanks for the advice this is my code for now
static Area FindOctantAndCreateQuadrant(Vector3d point,Vector3d bottom, Vector3d topDiagonal)
{
Vector3d newSize = (bottom - topDiagonal) / 2;
Vector3d Originalcenter = (bottom + topDiagonal) / 2;
int octantX = point.x < Originalcenter.x ? 0 : 1;
int octantY = point.y < Originalcenter.x ? 0 : 2;
int octantZ = point.z < Originalcenter.x ? 0 : 4;
int octant= octantX + octantY + octantZ;
Vector3d newOrigin;
int breakVale = 0;
switch (octant)
{
case 0:
newOrigin = bottom;
breakVale = 0;
break;
case 1:
newOrigin = bottom + newSize.xnn;
breakVale = 1;
break;
case 2:
newOrigin = bottom + newSize.nyn;
breakVale = 2;
break;
case 3:
newOrigin = bottom + newSize.xyn;
breakVale = 3;
break;
case 4:
newOrigin = bottom + newSize.nnz;
breakVale = 4;
break;
case 5:
newOrigin = bottom + newSize.xnz;
breakVale = 5;
break;
case 6:
newOrigin = bottom + newSize.nyz;
breakVale = 6;
break;
case 7:
newOrigin = bottom + newSize.xyz;
breakVale = 7;
break;
default:
newOrigin = bottom;
breakVale = 0;
break;
}
return new Area(newOrigin,newSize, breakVale);
}
Unless you got infinite time to check it this is my final result for now
copper copper
```PY
//Big thanks for the advice this is my code for now
static Area FindOctan...
Get rid of that breakVale variable. Notice how you have case 0: breakValue = 0, case 1: breakValue = 1 etc.
You can just do new Area(..., octant)
rancid mantle
so like it was before?
copper copper
No, it wasn't like that before
You were passing a constant number for each case
In each switch case you would just assign newOrigin
rancid mantle
oh yes I get it now
Some guy on some discord told me I was violating like a million programming rules lmao, but Ima leave it like that, a little change I made a seperate function for the octant, since Ima need it like that for not splitting the same Object twice
copper copper
I don't see any rule violation here lol what are they talking about
rancid mantle
he was talking about some SPR and OCP rules and thank you sm for the help I really like the code I have now