[STORM-1662] Reduce map lookups in send_to_eventlogger

[arunmahadevan]

Reducing map lookup in send_to_eventlogger can improve performance when when a spout emits in a tight loop.

[arunmahadevan]

@HeartSaVioR could you take a look ?

[HeartSaVioR]

+1 if it's same to arunmahadevan@7eae5ec.

I already applied this patch from #1217 and test, and it looks good.

Btw, I may need to have dedicated server during test, cause when last performance test for me this patch doesn't increase throughput when eventlogger=1 but this patch increase throughput when eventlogger=0.
(Tested with roshan's topology, I didn't test with your topology. I can't even work while benchmarking since I should run that for my work machine... ;( )

event logger executors=1, origin

uptime: 540 duration: 30 secs
spout emitted: 24016500 (total: 420641440) emitted/sec (in duration): 800550.0
bolt emitted: 24016620 (total: 420639920) emitted/sec (in duration): 800554.0
uptime: 570 duration: 30 secs
spout emitted: 24392600 (total: 445034040) emitted/sec (in duration): 813086.6666666666
bolt emitted: 24392480 (total: 445032400) emitted/sec (in duration): 813082.6666666666
uptime: 600 duration: 30 secs
spout emitted: 24524700 (total: 469558740) emitted/sec (in duration): 817490.0
bolt emitted: 24524620 (total: 469557020) emitted/sec (in duration): 817487.3333333334

event logger executors=0, origin

uptime: 541 duration: 30 secs
spout emitted: 27652560 (total: 441035220) emitted/sec (in duration): 921752.0
bolt emitted: 27653040 (total: 441034260) emitted/sec (in duration): 921768.0
uptime: 571 duration: 30 secs
spout emitted: 25150820 (total: 466186040) emitted/sec (in duration): 838360.6666666666
bolt emitted: 25150280 (total: 466184540) emitted/sec (in duration): 838342.6666666666
uptime: 601 duration: 30 secs
spout emitted: 25021760 (total: 491207800) emitted/sec (in duration): 834058.6666666666
bolt emitted: 25021860 (total: 491206400) emitted/sec (in duration): 834062.0

event logger executors=1, applied arun's patch

uptime: 541 duration: 30 secs
spout emitted: 24319300 (total: 426107880) emitted/sec (in duration): 810643.3333333334
bolt emitted: 24319140 (total: 426106420) emitted/sec (in duration): 810638.0
uptime: 571 duration: 30 secs
spout emitted: 24394800 (total: 450502680) emitted/sec (in duration): 813160.0
bolt emitted: 24395420 (total: 450501840) emitted/sec (in duration): 813180.6666666666
uptime: 601 duration: 30 secs
spout emitted: 24425140 (total: 474927820) emitted/sec (in duration): 814171.3333333334
bolt emitted: 24424720 (total: 474926560) emitted/sec (in duration): 814157.3333333334

event logger executors=0, applied arun's patch

uptime: 541 duration: 30 secs
spout emitted: 26572320 (total: 464465820) emitted/sec (in duration): 885744.0
bolt emitted: 26572680 (total: 464464560) emitted/sec (in duration): 885756.0
uptime: 571 duration: 30 secs
spout emitted: 26573580 (total: 491039400) emitted/sec (in duration): 885786.0
bolt emitted: 26573140 (total: 491037700) emitted/sec (in duration): 885771.3333333334
uptime: 601 duration: 30 secs
spout emitted: 26586520 (total: 517625920) emitted/sec (in duration): 886217.3333333334
bolt emitted: 26587000 (total: 517624700) emitted/sec (in duration): 886233.3333333334

Super odd result anyway, so I'd really like to see stable numbers.

[arunmahadevan]

@HeartSaVioR actually this patch has no effect with eventlogger=0. I assume you modified BasicTopology to emit a constant long. I also restart all the storm processes before each run and let it run for 12 mins and compare the last 10 min window stats.

[HeartSaVioR]

@arunmahadevan
No effect with eventlogger=0 is what I assume, so I'm saying that's weird...
I tested on roshan's topology with no modifying.
Maybe I need to clean up something with each test, or test with your topology.

[arunmahadevan]

I tested on roshan's topology with no modifying.

I also tried that and it was producing weird results, so I changed it to emit a constant long and restarted storm processes after each run.

[HeartSaVioR]

@arunmahadevan
I've just done with performance test, and the result is a bit different than what I expected.

This patch affects the performance even though event logger is set to 0 which I can't understand.
This performance test is done with your topology + modification for logging performance.
(https://gist.github.com/HeartSaVioR/0e2555633a5f7d12cb68)
I restarted all Storm processes for every test.

eventlogger-performance-test-STORM-1662-excel.xlsx

Since test is done with my dev. machine (MBP), I really would like to borrow a dedicated machine and try this performance test again.

[HeartSaVioR]

In result, sorry I changed my vote to +0 for now.
I think we should make the test result stable before taking any actions.

• test several times and take average or mean
• test longer
• test with dedicated (idle, not dev., no GUI) machine

If we can get the stable result before releasing 1.0.0 is in progress, we can include this as 1.0.0. But it shouldn't block releasing 1.0.0.