Diameter Stack: Improve Diameter Router (Performance & Footprint)

[GoogleCodeExporter]

This enhancement is intended to improve the performance and memory footprint of 
stack Diameter Router.

Original issue reported on code.google.com by brainslog on 13 Nov 2012 at 2:38

• Blocking: #21

Attachments:

• jdiameter_issue_22.patch

[GoogleCodeExporter]

Original comment by brainslog on 13 Nov 2012 at 2:39

Attachments:

• jdiameter_issue_23.patch

[GoogleCodeExporter]

Original comment by brainslog on 13 Nov 2012 at 2:49

• Now blocking: #21

[GoogleCodeExporter]

Some concerns regarding these changes:

- In requestEntryTable (Map), where a Long (Hop-by-Hop identifier) was used as 
key, a String is now being used (Session-Id + Hop-by-Hop + End-to-End). Besides 
the string concatenation performance issues, also the hashing should be 
impacting the performance. Not sure this is a good measure to avoid HbH 
collision.. which should not happen [that often];

- Removing (GC'ing) each message as it is sent seems also to be a perf 
penalty.. on the other hand, inserting into an almost empty map is much faster 
than to a crowded map. Do you have (if not, can you try?) concrete tests with 
this change only to see what is the perf difference ?

- Fully clearing the Request Table doesn't seem nice, as the risk of losing 
valid messages is very high, even if waiting 5000ms.. but with the GC in place, 
it should not grow to such point.. unless there is some kind of leak. Again, 
any comparison ?

Original comment by brainslog on 13 Nov 2012 at 3:32

[GoogleCodeExporter]

Hi,
In response to your 3 queries:

1) We were having collisions with HbH resulting in timeouts being experienced 
by clients (no response being sent as the router could not find a route). The 
concatenation of 3 strings is statistically insignificant compared to all the 
other processing.

2) Again, this has minimal performance impact on a per transaction basis as the 
map is fairly small (only storing inflight entries). The old mechanism of doing 
a periodic reaping was killing performance as under high throughput (say 3000 
tps), the reaping would happen fairly often and cause pauses that would cascade 
across the entire test and cause timeouts. This was exacerbated by the FSM 
being single threaded. Also, the reaping ended up sometimes removing inflight 
requests, again resulting in client timeouts.

3) I agree... (its a hack ;-) ) I only added that code in case there was some 
form of "leak". I never saw this code actually being used but kept it there 
just in case.

To be honest, the router changes were made in a bit of a rush when we were 
performance testing and saw the the router as it was could not deal with such 
high throughput (even with a huge map size). One these changes were made, it 
was as smooth as silk. I'm sure there may be more elegant solutions, but we 
left this one as you see it... cause it was working ;-)

Original comment by paul.car...@gtempaccount.com on 13 Nov 2012 at 7:41

[GoogleCodeExporter]

Hi Paul,

Thanks for your answers. Here's some more to add to discussion:

1) Regarding collisions, they may happen.. usually in a test scenario, not as 
much is a real-world scenario. While the string concatenation is heavy, related 
to using a long, it may not impact as much in the overall process. Also the 
hash process is heavier for String. But, I've ran some performance checks and 
it seems to behave fairly well.

2) Agree on this too, and after some performance tests I can confirm the change 
is for the better, and will scale better.

3) Right, as mentioned, this should not happen... only if there are leaks, so 
it's fairly safe to have it.

Thanks for these changes, we will integrate them on our code.

Original comment by brainslog on 19 Nov 2012 at 7:52

[GoogleCodeExporter]

This issue was updated by revision 7f6247cf767b.

Applied patch with some refactoring

Original comment by brainslog on 24 Nov 2012 at 3:49

• Changed state: Fixed