Implement post task gc hook

[simon-mo]

What do these changes do?

Add a decorator that perform manual GC after the task. It can help containment memory hungry task or pandas operator that does not deallocation memory correctly.

(https://stackoverflow.com/questions/35782929/pandas-groupby-memory-deallocation/37790738)

Related issue number

• passes git diff upstream/master -u -- "*.py" | flake8 --diff

[pschafhalter]

Why import here and not at the top of the file?

[kunalgosar]

This will cause import gc to be run every time that a function with the decorator is run

[simon-mo]

Because the ray worker process might not correctly import gc. (Especially working with cloudpickle)

[simon-mo]

@kunalgosar The python process will only import it once, the rest import is noop.

[kunalgosar]

I'm not sure if that's true, I think importing it at the beginning of the file would be fine. This seems to work well for the rest of the code?

[pschafhalter]

Is the latency still 100ms for gc'ing groupby on a smaller dataframe? Might be worth thinking about the speed-memory tradeoff.

[kunalgosar]

Can we potentially be more intelligent about when to run rc? Perhaps time the function run and as long as it is 5x the expected gc runtime, we gc? This way we cap speed performance regression to 20%.

[simon-mo]

Yes we should be more intelligent. I can add a basic heuristic for now, per your suggestion.

However, considering the use case, we are not faster than pandas on small dataframe.

The memory issue can be a problem for us already if the dataframe is few hundred MBs. For now, this is really about just making sure we can even do groupby without crashing (due to OOM).

[kunalgosar]

This will cause import gc to be run every time that a function with the decorator is run

[kunalgosar]

Can we potentially be more intelligent about when to run rc? Perhaps time the function run and as long as it is 5x the expected gc runtime, we gc? This way we cap speed performance regression to 20%.

[pschafhalter]

Using size of the dataframe could be another good heuristic since the concern for gc is memory. However, I'm fine with using time for now. Thoughts?

[devin-petersohn]

I think time makes a bit more sense right now because of the relative overhead of GC. We may in the future try to do this another way, but for now this looks good.

[devin-petersohn]

Looks great! Are there any other operations you think might benefit from this?

[devin-petersohn]

I think time makes a bit more sense right now because of the relative overhead of GC. We may in the future try to do this another way, but for now this looks good.

[kunalgosar]

Given the runtime heuristic, I think it may even make sense to apply this decorator on almost all functions. Potentially, we can profile a bit to categorize which ops would benefit the most from this.

I would guess blocks to cols/rows might benefit from this.

[devin-petersohn]

@kunalgosar blocks to cols/rows might especially benefit from this. For now, since we have good numbers for groupby memory usage, we can use it here (it doesn't affect runtime all that much).

Thanks @simon-mo! Great patch!