Welcome!

Join our community of MMO enthusiasts and game developers! By registering, you'll gain access to discussions on the latest developments in MMO server files and collaborate with like-minded individuals. Join us today and unlock the potential of MMO server development!

Join Today!

Celino Enum Iteration is NOT an optimization

Joined
Jun 5, 2010
Messages
567
Reaction score
598
Why is it that the system odinms had with the packet handlers is changed to iterate through the enums? It was a map before and it should run faster. Selecting from a hashmap should have on average constant time, while selecting with switch is at best linear time, and it is being looped from all the enums on top of that before even selecting. If there were 10 opcodes, MAYBE hashmap would be slower.

Haven't looked into java for a while but I believe for each works like this:
Code:
for (RecvPacketOpcode op : RecvPacketOpcode.values()) {
    ...
}
Code:
Iterator<RecvPacketOpcode> it = RecvPacketOpcode.values().iterator();
while (it.hasNext()) {
    RecvPacketOpcode op = (RecvPacketOpcode) it.next();
    ...
    it.remove();
}
which should not be faster.

Also, commands are cached using maps. Items and data are cached using maps. Why are packet handlers the only thing that aren't?


This is the odinms implementation:
Code:
if (map.containsKey(packetHeader)) handlePacket

This is the celino implementation:
Code:
for (RecvOpcode r : opcodes) {
    if (r.getValue() == packetHeader) {
        handlePacket(opcode)
        break;
    }
}
...
void handlePacket(packetHeader)
    switch (packetHeader) {
        case 1: handlePacket
        ...
        case 200: handlePacket
    }
 
Last edited:
Everything is possible~
Member
Joined
Jan 9, 2008
Messages
818
Reaction score
847
Re: Enum Iteration vs Map in Celino/Lithium sources

Map iteration is a slower than a simple switch. Most of the times, switches cannot compare strings (Java 6- and C++ for example), so maps are a 'good' replacement
 
Joined
Jun 5, 2010
Messages
567
Reaction score
598
Re: Enum Iteration vs Map in Celino/Lithium sources

Map iteration is a slower than a simple switch. Most of the times, switches cannot compare strings (Java 6- and C++ for example), so maps are a 'good' replacement

That's not relevant to comparing map lookup (average time 1) and iteration, which is always n. Also we're not switching strings either. Rather we're switching opcodes.

edit:
compare
if (map.get(key) != null) with for(Opcode op in enum.values()). Assuming a good hash, there is no way the second is faster on average, or even beneficial to memory as the opcode objects were tiny. This is just checking whether we have a handler in the first place. After the loop happens, it does something like if (opcode == asdf) handleAsdf elseif (opcode == asdfg) handleAsdfg

Thread title changed to be more clear
 
Last edited:
Joined
Apr 5, 2008
Messages
663
Reaction score
537
The most efficient way to handle opcode handlers is as follows:
Code:
//Initialize this at server startup. All values which do not have a handler should be set to a default handler which throws some sort of error.
function<void(packet*)> handlers[0x10000];
//When handling a packet
handlers[opcode](somepacket);
This provides a truly constant lookup time, rather than a merely constant amortized time provided by a hash. As well, it ends up compiling to only a couple asm instructions while any sort of hash or tree can easily involve hundreds of instructions. Just because hash-based containers provide constant lookup time does not mean that lookup time is fast.
In this case both Celino and OdinMS provide subpar implementations for opcode handling.
 
Joined
Jun 5, 2010
Messages
567
Reaction score
598
The most efficient way to handle opcode handlers is as follows:
Code:
//Initialize this at server startup. All values which do not have a handler should be set to a default handler which throws some sort of error.
function<void(packet*)> handlers[0x10000];
//When handling a packet
handlers[opcode](somepacket);
This provides a truly constant lookup time, rather than a merely constant amortized time provided by a hash. As well, it ends up compiling to only a couple asm instructions while any sort of hash or tree can easily involve hundreds of instructions. Just because hash-based containers provide constant lookup time does not mean that lookup time is fast.
In this case both Celino and OdinMS provide subpar implementations for opcode handling.

That is true, and I must have misread OdinMS code but it is actually done with something similar to that (not exactly but close).


Anyway... I noticed GMS does something similar to the way Celino does it (from the KMST leaked code). Why is this?
 
Initiate Mage
Joined
Feb 17, 2013
Messages
1
Reaction score
1
That seems to be one of my old build that was leaked out. Unfortunately due to Asiasoft, I can't reveal much of the Celino (v115) Source code as much as I wish to.

However for educational purpose, here's my latest build dated June 2012 (The last time that it was last changed)
Code:
    @Override
    public void messageReceived(final IoSession session, final Object message) throws Exception {
 final SeekableLittleEndianAccessor slea = new GenericSeekableLittleEndianAccessor(new ByteArrayByteStream((byte[]) message));
 final RecvPacketOpcode code = Header.get(slea.readShort());
 if (code != null) {
     final Client c = (Client) session.getAttribute(StringPool.CLIENT_KEY);
     if (ServerConstants.enableHandledPacketLogging) {// Debugging
  if (!code.isIgnorePacketSpamOrLog()) {
      System.out.println(String.format("[%s] %s\nString : %s", code.name(), HexTool.toString((byte[]) message), HexTool.toStringFromAscii((byte[]) message)));
  }
     }
     if (code.NeedsChecking()) {
  if (!c.isLoggedIn()) {
      session.close(true);
      return;
  }
  /*
   * if ((code.isCSOperation() && type == ServerType.CHANNEL) ||
   * (!code.isCSOperation() && type != ServerType.CHANNEL)) {
   * return; }
   */
     }
     final long cTime = System.currentTimeMillis();
     final long Differences = cTime - c.LastCapturedTimeMillis_500MSThreshold;
     if (Differences < 500) { // within 500ms
  if (!code.isIgnorePacketSpamOrLog() && !c.FlagPendingDisconnection) { // Not move life, mob, summon, dragon
      c.PacketSpamCountWithinHalfSecond++;
      // 70 should be the acceptable level, but we will test with 200 first to make sure it doesn't affect laggers.
      if (c.PacketSpamCountWithinHalfSecond > 200) { // Spam > 70 packet within 500ms = dc.
   c.FlagPendingDisconnection();
   ServerLog.RegisterForLogging(ServerLogType.PacketSpam,
    String.format("[%s 0x%s] CharOrAccId : %s, Field : %s, Count : %d\nData : %s\nString : %s",
    code.name(),
    Integer.toHexString(code.getValue()),
    c.getPlayer() != null ? c.getPlayer().getName() : "Accid=" + String.valueOf(c.getAccID()),
    c.getPlayer() != null ? MapleMapFactory.getFullMapName(c.getPlayer().getMapId()) : "-1",
    c.PacketSpamCountWithinHalfSecond,
    HexTool.toString((byte[]) message),
    HexTool.toStringFromAscii((byte[]) message)));
      }
  }
     } else {
  c.LastCapturedTimeMillis_500MSThreshold = cTime;
  c.PacketSpamCountWithinHalfSecond = 0;
     }
     // This is also used throughout the packet handler to determine the current time instead of calling System.currentTimeMillis() again
     c.LastCapturedTimeMillis = cTime;
//     System.out.println("Differences : "+(Differences)+", count : "+c.PacketSpamCountWithinHalfSecond+", cTime : " + cTime);
     handlePacket(code, slea, c, type);
 } else if (ServerConstants.enableUnhandledPacketLogging) { // Console output part for debugging
     System.out.println(String.format("[Unhandled Packet] %s\nString : %s", HexTool.toString((byte[]) message), HexTool.toStringFromAscii((byte[]) message)));
 }
    }

or pastebin for better formatting :

It may not be the best way of doing it, but as you can see, I'm learning as I code :p: I've always been experimenting with different ways of doing things.

--
DO not ask me anything about maplestory btw.. I'm no longer working on it.
 
Last edited:
Skilled Illusionist
Joined
May 28, 2011
Messages
380
Reaction score
38
That seems to be one of my old build that was leaked out. Unfortunately due to Asiasoft, I can't reveal much of the Celino (v115) Source code as much as I wish to.

However for educational purpose, here's my latest build dated June 2012 (The last time that it was last changed)
Code:
    @Override
    public void messageReceived(final IoSession session, final Object message) throws Exception {
 final SeekableLittleEndianAccessor slea = new GenericSeekableLittleEndianAccessor(new ByteArrayByteStream((byte[]) message));
 final RecvPacketOpcode code = Header.get(slea.readShort());
 if (code != null) {
     final Client c = (Client) session.getAttribute(StringPool.CLIENT_KEY);
     if (ServerConstants.enableHandledPacketLogging) {// Debugging
  if (!code.isIgnorePacketSpamOrLog()) {
      System.out.println(String.format("[%s] %s\nString : %s", code.name(), HexTool.toString((byte[]) message), HexTool.toStringFromAscii((byte[]) message)));
  }
     }
     if (code.NeedsChecking()) {
  if (!c.isLoggedIn()) {
      session.close(true);
      return;
  }
  /*
   * if ((code.isCSOperation() && type == ServerType.CHANNEL) ||
   * (!code.isCSOperation() && type != ServerType.CHANNEL)) {
   * return; }
   */
     }
     final long cTime = System.currentTimeMillis();
     final long Differences = cTime - c.LastCapturedTimeMillis_500MSThreshold;
     if (Differences < 500) { // within 500ms
  if (!code.isIgnorePacketSpamOrLog() && !c.FlagPendingDisconnection) { // Not move life, mob, summon, dragon
      c.PacketSpamCountWithinHalfSecond++;
      // 70 should be the acceptable level, but we will test with 200 first to make sure it doesn't affect laggers.
      if (c.PacketSpamCountWithinHalfSecond > 200) { // Spam > 70 packet within 500ms = dc.
   c.FlagPendingDisconnection();
   ServerLog.RegisterForLogging(ServerLogType.PacketSpam,
    String.format("[%s 0x%s] CharOrAccId : %s, Field : %s, Count : %d\nData : %s\nString : %s",
    code.name(),
    Integer.toHexString(code.getValue()),
    c.getPlayer() != null ? c.getPlayer().getName() : "Accid=" + String.valueOf(c.getAccID()),
    c.getPlayer() != null ? MapleMapFactory.getFullMapName(c.getPlayer().getMapId()) : "-1",
    c.PacketSpamCountWithinHalfSecond,
    HexTool.toString((byte[]) message),
    HexTool.toStringFromAscii((byte[]) message)));
      }
  }
     } else {
  c.LastCapturedTimeMillis_500MSThreshold = cTime;
  c.PacketSpamCountWithinHalfSecond = 0;
     }
     // This is also used throughout the packet handler to determine the current time instead of calling System.currentTimeMillis() again
     c.LastCapturedTimeMillis = cTime;
//     System.out.println("Differences : "+(Differences)+", count : "+c.PacketSpamCountWithinHalfSecond+", cTime : " + cTime);
     handlePacket(code, slea, c, type);
 } else if (ServerConstants.enableUnhandledPacketLogging) { // Console output part for debugging
     System.out.println(String.format("[Unhandled Packet] %s\nString : %s", HexTool.toString((byte[]) message), HexTool.toStringFromAscii((byte[]) message)));
 }
    }

or pastebin for better formatting :

It may not be the best way of doing it, but as you can see, I'm learning as I code :p: I've always been experimenting with different ways of doing things.

--
DO not ask me anything about maplestory btw.. I'm no longer working on it.
Not a fan of the structure, since I still prefer the older methods, such as in old Odin/Moople, but thanks for taking the time to show us some of your progress as a coder. Good luck with your future projects!
 
Joined
Jun 5, 2010
Messages
567
Reaction score
598
The idea was that the old way of doing it is actually faster than this way. Array lookup is faster than linear search through if statements. It's not that I prefer the old way of doing it, but rather that it is actually faster.
 
<3
Joined
Feb 4, 2011
Messages
481
Reaction score
123
I am no expert here(I am just your typical Computer Science student in high school)...but why use linear search? Isn't binary search faster in the long run?
 
Delta
Member
Joined
Apr 4, 2008
Messages
951
Reaction score
305
The idea was that the old way of doing it is actually faster than this way. Array lookup is faster than linear search through if statements. It's not that I prefer the old way of doing it, but rather that it is actually faster.
Have you bench marked this or is this just an assumption? :O I'm curious because Odin is structured with a version of Java which seems to be rather obsolete with the new improvements to it over the years. Or rather additions / changes.
 
<3
Joined
Feb 4, 2011
Messages
481
Reaction score
123
Have you bench marked this or is this just an assumption? :O I'm curious because Odin is structured with a version of Java which seems to be rather obsolete with the new improvements to it over the years. Or rather additions / changes.

I agree with Sparks. That is an interesting question. And, here is another question to add on top of his:

First of all, keep in mind, my understanding of MapleStory code/Java knowledge is rather limited.

Now then, just to clarify that I understand you correctly, 'array lookup' is basically searching for an element in the array, correct? If so, could you clarify what search method you'd be using for searching an array?

From my understanding, you would only be using linear search on an array if the array is unsorted. And if it is sorted, binary search would be the way to go, since it would be a lot faster. This is due to the fact that with linear search, it's worst case would be running 'n' times, where 'n' is the number of elements in the array. And it's average would be half of that. Meanwhile, binary search would also be the same, but it's average would be log n (which is a whole lot better)
 
Last edited:
Joined
Jun 5, 2010
Messages
567
Reaction score
598
I agree with Sparks. That is an interesting question. And, here is another question to add on top of his:

First of all, keep in mind, my understanding of MapleStory code/Java knowledge is rather limited.

Now then, just to clarify that I understand you correctly, 'array lookup' is basically searching for an element in the array, correct? If so, could you clarify what search method you'd be using for searching an array?

From my understanding, you would only be using linear search on an array if the array is unsorted. And if it is sorted, binary search would be the way to go, since it would be a lot faster. This is due to the fact that with linear search, it's worst case would be running 'n' times, where 'n' is the number of elements in the array. And it's average would be half of that. Meanwhile, binary search would also be the same, but it's average would be log n (which is a whole lot better)

The array is sorted. Look at how it's done in Packet Processor.
Packet[opcodeid] = new Handler();

Now to get the handler you do Packet[slea.readShort] which is for sure faster than doing if (packetId == ??) else if (packetId == ??). See retep's post for more details. I misread the code at first but this is how odinms actually does it.

Binary search works if the array wasn't constructed like above. There is no need to search binary if you try to find Packet 100 when you can do Packet[100]. Constant time search would be the best and since this is an array, there are barely any lines of assembly needed to get arrays since they are all stored in the same order and array calls are relatively fast compared to list and they are certainly faster than multiple if/then statements.

Binary search wouldn't really make sense either here with Celino because their things are not sorted and there's no way to go left/right when you are at the center when there are higher values on either side.

Odin is structured with java 6 which is not old at all. This method should be used cross platform anyway as it really is the fastest. This is also using primitive types (same reason why System.arrayCopy is faster than copying manually even though I'm sure it does do very similar thing. Maybe the compiler optimizes better)
 
Back
Top