Optimizing heap utilization

#11 Merged at b88a2e8
Repository
vengadan-seunjeon
Branch
heap_optimize
Repository
seunjeon
Branch
heap_optimize
Author
  1. Vengadanathan
Reviewers
Description

Description:

seunjeon is occupying heap of around 560 MB Initial optimization done by us reduced the heap utilization of the plugin to ~ 174 MB (around 68% reduction) in heap usage.

Profiling and metrics:

  • Each query (when compared to unoptimized version of the plugin) will take additional .8-1ms (on an average) to execute because of decompression that we do when reading the feature array from memory.
  • seunjeon lexiconDict is occupying heap of around 174 MB instead of existing 560 MB

Testing: All existing UT's and newly added UT's passed

Comments (10)

  1. 영호 유

    I tested the performance of your code.

    Considering the indexing-time, I tried to do a rather long document.

    It took 15~20 times more time than un-patched.

    elapsedTime =  7,326,618 us -> seunjeon
    elapsedTime =  9,018,791 us -> seunjeon
    elapsedTime =  8,779,136 us -> seunjeon
    
    elapsedTime = 141,811,083 us -> seunjeon-snappy
    elapsedTime = 143,032,741 us -> seunjeon-snappy
    elapsedTime = 153,126,649 us -> seunjeon-snappy
    

    The example document is this.

    Unfortunately, at this time, It seems unreasonable to replace it. there seems to lose more than we get.

    It may be available on systems with low memory.

    I think we should be more concerned. (optional use if necessary..)

    Are you actually using the modified version yourself?

    --

    CC: @bibreen

  2. Vengadanathan author

    Hi,

    Thanks for reviewing.

    I agree that our optimization might not fit for all usecase. We also have requirement to run this plugin on smaller instance type hence we were aggressive in reducing heap.

    I made some change now to make it suitable for all usecases, where i tweaked some optimization so that the observed performance issue wont much. (post this change optimization of heap would be 59%)

    Kindly check the latest changes made to this PR.

    Analysis post new fix:

    patched:
    elapsedTime = 36328496 us
    elapsedTime = 35228325 us
    elapsedTime = 27632573 us
    elapsedTime = 30770790 us
    
    master:
    elapsedTime = 10566834 us
    elapsedTime = 9211610 us
    elapsedTime = 25522672 us
    elapsedTime = 9977699 us
    elapsedTime = 14568733 us
    

    Let us know if it looks fine. If not what is the acceptable amount of performance degradation we are fine with ? Additionally you can tweak some optimization based to change made if needed.

  3. 영호 유

    Thank you for your tuning. I had enough understanding of your needs.

    I have some ideas.

    It seems a bit difficult to give up performance. So I want to offer compression as an option.

    If you don’t think badly, I want to fix it as follows.

    It basically provides a compressed dictionary and gives users the option to choose whether to use uncompressed "Morpheme" or directly "CompressedMorpheme" at initialization time.

    If the initial decompression time is too long, we might think of offering two versions separately.

    What do you think about this?

    If you agree, I'll write additional code after merge it.

  4. Vengadanathan author

    I agree with you. Providing the option to the user to choose between memory optimized dictionary or not would helpful. You can merge in the change and make it optional as you mentioned i am fine with it.

    Thank you very much.

  5. 영호 유

    I do not know if this process will be clean. If not, I will think about separating the dictionary.

  6. 영호 유

    @vengadan_amzn

    Could you PR to another branch? i can't change the target branch..

    if you possible, new branch is good. if not, develop branch. please.

  7. 영호 유

    Hello @vengadan_amzn

    I've done some modifications to the code. I need to review the results. (Look at the heap_optimize branch.)

    Please confirm that there is no major memory problem. Please check it against the existing compression rate.

    And if you have any other opinions, please tell me.

    To describe the altered part

    Dictionary compression removal. As the pressure is reduced by jar

    Snappy compression removal of CompressedMorpheme.surface It consumes most of the execution time and seems to have little compression effect because of its short length.

    Adding "compress" option to elasticsearch plugin

    In many ways, your ideas have been very helpful. If we have time, we can increase memory efficiency in more areas.

  8. 영호 유

    @vengadan_amzn

    And I have optimized memory to some extent in the basic morpheme.

    Please also check whether your system can run in the uncompressed mode.

    Changes to BasicMorpheme are shown below.

    • Change the analysis results from list to stream.
    • Change the "feature" from an Array[String] to a String.

    In the distant future, we will continue to optimize memory. And we want to keep only one mode.