Raised This Month: $12 Target: $400
 3% 

Is this execution correct?


Post New Thread Reply   
 
Thread Tools Display Modes
Author Message
Krystek.
Member
Join Date: May 2022
Old 07-21-2022 , 03:44   Is this execution correct?
Reply With Quote #1

Is this execution correct?

PHP Code:
    new damage_better_items random_num(312);
    new 
damage_items         random_num(16);

    new 
perks_id[] = { 1234713 };

    for( new 
isizeof(perks_id); i++ ) {
        if( 
cod_get_user_perk(vid) == perks_id[i] ) {
            
log_amx("Better Durability: %i | PerkID %i"durability_perks[vid], perks_id[i]);
            
durability_perks[vid] -= (durability_perks[vid] > damage_better_items)? damage_better_itemsdurability_perks[vid];
            
log_amx("Better Durability: %i | PerkID %i"durability_perks[vid], perks_id[i]);

            return 
PLUGIN_CONTINUE;
        } 
    }

    for( new 
isizeof(perks_id); i++ ) {
        if( 
cod_get_user_perk(vid) != perks_id[i] ) {
            
log_amx("Durability: %i"durability_perks[vid], perks_id[i]);
            
durability_perks[vid] -= (durability_perks[vid] > damage_items)? damage_itemsdurability_perks[vid];
            
log_amx("Durability: %i"durability_perks[vid], perks_id[i]);

            return 
PLUGIN_CONTINUE;
        } 
    } 
The code is pretty loose so don't worry about log_amx and the like.

Last edited by Krystek.; 07-21-2022 at 03:45.
Krystek. is offline
damage220
Member
Join Date: Jul 2022
Location: Ukraine
Old 07-21-2022 , 09:07   Re: Is this execution correct?
Reply With Quote #2

What do you mean by correct? It depends. Just compile and run it.
1. You should never use sizeof( in a loop condition. Create a variable and store the size in it.
2. To speed up execution, move non-dynamic arrays out to global scope (precache), make them constant.
3. Try to avoid magic numbers, use constants instead.

Last edited by damage220; 07-22-2022 at 14:59.
damage220 is offline
fysiks
Veteran Member
Join Date: Sep 2007
Location: Flatland, USA
Old 07-22-2022 , 00:29   Re: Is this execution correct?
Reply With Quote #3

Quote:
Originally Posted by damage220 View Post
1. You should never use sizeof( in a loop condition. Create a variable and store the size in it.
That's the first time I've ever heard this in the 15 years I've been around here. I was under the impression that sizeof was evaluated at compile-time (except in specific use cases). I've only ever heard people say that you shouldn't use a function (sizeof is not a function, it's an operator) in a for or while loop's definition if the value will not change during the loop.

Quote:
Originally Posted by damage220 View Post
2. To speed up execution, move static arrays out to global scope, precache them.
This is not good advice. If something is only used in that function and has no usefulness to other areas of code, it should not be global (making a local variable/constant global just pollutes the global namespace).

If it's relatively small and the function is not called very often then it's generally negligible to declare it as "new". If it never changes then it should be declared as a constant ("const"). If it's a large variable (e.g. an array with a lot of cells) and it's not constant, it should be declared as a static variable ("static").
__________________
fysiks is offline
HamletEagle
AMX Mod X Plugin Approver
Join Date: Sep 2013
Location: Romania
Old 07-22-2022 , 07:05   Re: Is this execution correct?
Reply With Quote #4

1. fysiks is correct. sizeof is an operator and its result is known at compile time. The compilr replaces it with the actual value in the generated binary.
There is no runtime overhead.

Things are different with something like ArraySize, which is a function. It should be cached before the loop to avoid the runtime overhead of calling the function(pushing, popping the stack, jumping in a different code location) with each loop iteration.

2. I again must agree with fysiks. He explained it well so I will not reiterate. I just want to say that static is not meant to be used as an optimization technique. It can be used as such in very specific scenarios, but this is a side effect of how static works and not its main purpose.
__________________

Last edited by HamletEagle; 07-22-2022 at 07:11.
HamletEagle is online now
damage220
Member
Join Date: Jul 2022
Location: Ukraine
Old 07-22-2022 , 15:14   Re: Is this execution correct?
Reply With Quote #5

I am sorry for the first advice. Though I knew that sizeof in C is calculated at compile time, I had no luck to find any confirmation regarding PAWN.

"If it's relatively small and the function is not called very often"
Why did you decide that the function is not called very often? In fact I do not see any function at all, just the code. There is no function signature or event hook declared.

"This is not good advice. If something is only used in that function and has no usefulness to other areas of code, it should not be global (making a local variable/constant global just pollutes the global namespace)."
If a variable is used only in one function, I agree it should be declared in that function.
If a constant is never changes, like PI, it can be declared in a function.
But when a constant is subject to change I would prefer to have it global, just for convenience for future edits. I was kind of thinking that perks_id is subject to change, and so I would move it out of the function to simplify future edits and to reduce the number of memory allocations.
damage220 is offline
fysiks
Veteran Member
Join Date: Sep 2007
Location: Flatland, USA
Old 07-23-2022 , 00:04   Re: Is this execution correct?
Reply With Quote #6

Quote:
Originally Posted by Krystek. View Post
Is this execution correct?

PHP Code:
    new damage_better_items random_num(312);
    new 
damage_items         random_num(16);

    new 
perks_id[] = { 1234713 };

    for( new 
isizeof(perks_id); i++ ) {
        if( 
cod_get_user_perk(vid) == perks_id[i] ) {
            
log_amx("Better Durability: %i | PerkID %i"durability_perks[vid], perks_id[i]);
            
durability_perks[vid] -= (durability_perks[vid] > damage_better_items)? damage_better_itemsdurability_perks[vid];
            
log_amx("Better Durability: %i | PerkID %i"durability_perks[vid], perks_id[i]);

            return 
PLUGIN_CONTINUE;
        } 
    }

    for( new 
isizeof(perks_id); i++ ) {
        if( 
cod_get_user_perk(vid) != perks_id[i] ) {
            
log_amx("Durability: %i"durability_perks[vid], perks_id[i]);
            
durability_perks[vid] -= (durability_perks[vid] > damage_items)? damage_itemsdurability_perks[vid];
            
log_amx("Durability: %i"durability_perks[vid], perks_id[i]);

            return 
PLUGIN_CONTINUE;
        } 
    } 
The code is pretty loose so don't worry about log_amx and the like.
The thing that stands out to me about this is that you have code execution for all every cell in perks_id[] (because the if statements are opposite so one or the other code blocks will get executed) but you're being inefficient about it by using two loops.

You can do this in a single loop by just looping through all cells of the array and just using an if-then-else to determine which code you execute on each iteration.

P.S. While I understand that it's only part of your debugging, it's probably a good thing to point out so that you get better at seeing your coding mistakes in general: your log_amx() functions in your second loop should be throwing errors because you pass two arguments but the format string only has one.

_____________________________________________ _____


Quote:
Originally Posted by damage220 View Post
I am sorry for the first advice. Though I knew that sizeof in C is calculated at compile time, I had no luck to find any confirmation regarding PAWN.
I too was looking in the Pawn Language Guide to see if I could find it stated as such but I could not. This is why I was less absolute about my words on this one.

Quote:
Originally Posted by damage220 View Post
Why did you decide that the function is not called very often? In fact I do not see any function at all, just the code. There is no function signature or event hook declared.
My advice was not 100% based on the OP's code because I was primarily correcting what you said so that people don't accidentally use it.

You mentioned moving a variable to the global scope as an optimization. While that would potentially optimize it (depending on the specific code) it's not the only way to do it. The reason you said to move it to the global scope was so that it didn't have to dynamically re-allocate the memory each time the function is called. Well, you can do this same thing while keeping it in the function scope by declaring it as static.

Regarding if a function (or rather a block of code) is called often, that's because of the dynamic allocation functionality. If it's called often, it has to get re-allocated a lot. If it's big then this re-allocation becomes more expensive. So, making it static (or global) removes the re-allocation and thus reduce processor time required. For small variables or code that is rarely called, the processor time required stays relatively low so it's not really worth optimizing it (i.e. it's a negligible performance cost).

Quote:
Originally Posted by damage220 View Post
If a variable is used only in one function, I agree it should be declared in that function.
If a constant is never changes, like PI, it can be declared in a function.
But when a constant is subject to change I would prefer to have it global, just for convenience for future edits. I was kind of thinking that perks_id is subject to change, and so I would move it out of the function to simplify future edits and to reduce the number of memory allocations.
Whether or not it's a constant (constants don't change) or a variable does not dictate in what scope it should be declared. You should declare the constant or variable in the proper scope for it's use case. If a constant or variable is only useful to the code within a specific function, it should be declared in the function.

Whether or not the perks_id array will change is not clear in the code posted by the OP. If it's within a function and is not used outside the function, it makes sense to declare it inside the function. If the OP decides to change how perks_id is used, the best scope for that variable can be re-evaluated.

Granted, you can declare everything globally if you really wanted to as long as you don't have naming conflicts with any other variable in the entire code. However, this can make the code unnecessarily complex. Scoping variables helps makes code simpler and more modular.

_____________________________________

We probably don't need to go any further with this part of the discussion. We don't want to hijack the thread any more .
__________________

Last edited by fysiks; 07-23-2022 at 00:05.
fysiks is offline
Krystek.
Member
Join Date: May 2022
Old 07-23-2022 , 02:42   Re: Is this execution correct?
Reply With Quote #7

Quote:
Originally Posted by fysiks View Post
The thing that stands out to me about this is that you have code execution for all every cell in perks_id[] (because the if statements are opposite so one or the other code blocks will get executed) but you're being inefficient about it by using two loops.

You can do this in a single loop by just looping through all cells of the array and just using an if-then-else to determine which code you execute on each iteration.

P.S. While I understand that it's only part of your debugging, it's probably a good thing to point out so that you get better at seeing your coding mistakes in general: your log_amx() functions in your second loop should be throwing errors because you pass two arguments but the format string only has one.
PHP Code:
    new damage_better_items random_num(312);
    new 
damage_items         random_num(16);

    new 
perks_id[] = { 1234713 };

    for( new 
isizeof(perks_id); i++ ) {
        if( 
cod_get_user_perk(vid) == perks_id[i] ) {
            
log_amx("Better Durability: %i | PerkID %i"durability_perks[vid], perks_id[i]);
            
durability_perks[vid] -= (durability_perks[vid] > damage_better_items)? damage_better_itemsdurability_perks[vid];
            
log_amx("Better Durability: %i | PerkID %i"durability_perks[vid], perks_id[i]);

            return 
PLUGIN_CONTINUE;
        } else {
            
log_amx("Durability: %i"durability_perks[vid], perks_id[i]);
            
durability_perks[vid] -= (durability_perks[vid] > damage_items)? damage_itemsdurability_perks[vid];
            
log_amx("Durability: %i"durability_perks[vid], perks_id[i]);

            return 
PLUGIN_CONTINUE;
        }
    } 
By doing it this way, only the content after the else is executed.

Last edited by Krystek.; 07-23-2022 at 02:43.
Krystek. is offline
fysiks
Veteran Member
Join Date: Sep 2007
Location: Flatland, USA
Old 07-23-2022 , 03:41   Re: Is this execution correct?
Reply With Quote #8

You forgot to remove the return statements. Remove them both. Optionally, you can put it after the for loop (depending on what kind of function this is). I probably forgot to mention this part. Sorry about that.
__________________

Last edited by fysiks; 07-23-2022 at 03:41.
fysiks is offline
Krystek.
Member
Join Date: May 2022
Old 07-23-2022 , 04:00   Re: Is this execution correct?
Reply With Quote #9

Quote:
Originally Posted by fysiks View Post
You forgot to remove the return statements. Remove them both. Optionally, you can put it after the for loop (depending on what kind of function this is). I probably forgot to mention this part. Sorry about that.
Removing a return does something like this.
https://prnt.sc/I5tY7-cFzfht

Last edited by Krystek.; 07-23-2022 at 04:04.
Krystek. is offline
fysiks
Veteran Member
Join Date: Sep 2007
Location: Flatland, USA
Old 07-23-2022 , 04:03   Re: Is this execution correct?
Reply With Quote #10

Quote:
Originally Posted by Krystek. View Post
Removing the return does not do anything.
Did you remove both of them? It also looks like you haven't fixed the amx_log arguments that I mentioned. I think that will probably cause an error and prevent the loop from working like it should.
__________________
fysiks is offline
Reply


Thread Tools
Display Modes

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off

Forum Jump


All times are GMT -4. The time now is 08:29.


Powered by vBulletin®
Copyright ©2000 - 2024, vBulletin Solutions, Inc.
Theme made by Freecode