• November 29, 2022, 09:29:47 am

Author Topic: Warrior's code suggestions  (Read 2456 times)

warrior

  • Sr. Member
  • ******
  • Posts: 409
  • Jesus Christ.
Warrior's code suggestions
« on: March 11, 2011, 10:36:04 am »
Review for http://code.google.com/p/reticle/source/browse/trunk/Reticle/Bot/Bot.cs:
- You're not unregistering your event handlers (you += but never -=) after you dispose of the underlying BN# object. This means that BN# client object will never be garbage collected and will cause a memory leak. If you ever try to completely unload a profile (therefore destroying its associated Bot instance) you will run into trouble.

- You're concating strings together when strings are immutable. You should use string.Format(..) (Look it up on MSDN) instead of "this" + "that" + "them" for your building of messages to AddChat. string.Format() internally uses StringBuilder which performs much faster, especially if you're going to be doing it a lot during floods.

Every time you use + in a string operation ("this" + "that") you're actually creating three instances of a string. One for "this", one for "that", and one for "this that". string.Format() only creates one. An example usage would be
Code: [Select]
string finalOutput = string.Format("{0} {1}", this, that);

Now the performance of this may not be too significant immediately, but for example if you're processing a command, and AddChatting under a variable sized loop, from my experience, the performance is much better.

- You should convert your string static helper functions (ConvertHTML and such) to .NET Extension Methods. Look it up on MSDN, its really easy to do, and will make your code cleaner:

Instead of Bot.ConvertHTML("...."); you can do string cleanedText = "myhtmltag".ConvertHTML();

Now for
http://code.google.com/p/reticle/source/browse/trunk/Reticle/Bot/CDKeyManager.cs:

- You should consider XML serialization, its faster, and you'll fall inlove with it after you learn it.

http://code.google.com/p/reticle/source/browse/trunk/Reticle/Bot/Commands.cs:

- You should consider a Dictionary<K,V> where K is a string and V is a delegate. This has the benefit of being O(1) or a constant look up, vs a variable look up of a string. Its not important now since you have like five commands, but if you ever start adding a ton of commands, you will see huge performance wins during flood scenarios or scenarios where response time to commands is critical.

That's pretty much all I have for now, but if I see anything else I'll add to this thread along with rationale. You should also consider learning the Task Parallel Library to get scalability of up to 256 cores.
In capitalist America, bank robs you.

Choosing to code in an unmanaged language/platform is like choosing a hotel where you have to clean your own room.

When C++ is your hammer, everything starts to look like your thumb

pikachu

  • Administrator
  • Hero Member
  • *******
  • Posts: 3,344
Re: Warrior's code suggestions
« Reply #1 on: March 11, 2011, 02:51:21 pm »
Simply because I'm interested in the topic:

O(1)

Isn't this not technically true?  I remember hearing that they can be O(n) in some scenarios, and doesn't the key size matter when you're hashing?  Larger keys will require (although probably negligible) more time to hash, no?

Also, doesn't the performance ride mainly on the load factor?

warrior

  • Sr. Member
  • ******
  • Posts: 409
  • Jesus Christ.
Re: Warrior's code suggestions
« Reply #2 on: March 11, 2011, 06:07:30 pm »
Isn't this not technically true?  I remember hearing that they can be O(n) in some scenarios, and doesn't the key size matter when you're hashing?  Larger keys will require (although probably negligible) more time to hash, no?

Also, doesn't the performance ride mainly on the load factor?

Yes, most of the data structures are not worst-case O(1) but rather amoritized O(1) so its really "O(1)*" where its "on average" constant lookup. And I'd imagine so re: the hashing perf impact.

Yes to your second question as well, the performance win becomes more desirable in a heavily hit subroutine. Which is something a command processor (as well as packet processor) has the potential for being. At any rate, even if it never does routinely see it, I'd think the price of instantiating the Dictionary<K,V> is ideally only done one time per execution of the program, so it is is a balance between effective load time and performance under load.
This post has been thanked 1 time pikachu
In capitalist America, bank robs you.

Choosing to code in an unmanaged language/platform is like choosing a hotel where you have to clean your own room.

When C++ is your hammer, everything starts to look like your thumb

pikachu

  • Administrator
  • Hero Member
  • *******
  • Posts: 3,344
Re: Warrior's code suggestions
« Reply #3 on: March 11, 2011, 07:20:23 pm »
Thanks :)

 

newBalance by DzinerStudio