Parser performance not optimal ~1min for a 24MB file about redis-rdb-tools HOT 15 OPEN

Did some profiling and a quick patch with: https://github.com/teepark/python-lzf resulting in x2 performance boost.

from redis-rdb-tools.

@yoav-steinberg : Thanks for taking time to investigate this issue!

I don't like adding a dependency to the project. Let me investigate if there is a way to conditionally include the library, so that people who don't want to install the dependency can still use rdb-tools.

from redis-rdb-tools.

You can also consider including the c files from the liblzf directly in redis-rdb-tools instead of adding a dependency. It is fairly common to have liblzf files included inside a larger project (actually redis does this!).

from redis-rdb-tools.

Simplistic patch here: https://github.com/jvtm/redis-rdb-tools/tree/lzf-speedup

Not creating a pull request just yet, I want to test this with real fresh dumps first. The related unit tests pass, but I didn't check if error reporting on invalid values behaves the same.

from redis-rdb-tools.

@sripathikrishnan any thoughts on including @jvtm's patch? It doesn't require python-lzf, but uses it if its there.

from redis-rdb-tools.

bump. Any chance on getting this? Parsing a 10g backup for me is brutal.

from redis-rdb-tools.

Wow, didn't even remember this one... Not working anymore on the project where this was required. Here's the exact tiny commit: jvtm@fdd8134 (failed to include this issue in commit message)

from redis-rdb-tools.

I dug around the code and noticed this commit introduced the lzf optimization.

from redis-rdb-tools.

@billcrook the current code only uses the lzf optimization if you have the native library installed. maybe you just need to do pip install lzf?

does the commit @jvtm mentioned changes anything? seem to me that it does the same thing the current version already does. please let me know if i'm missing anything.

from redis-rdb-tools.

@billcrook the current code only uses the lzf optimization if you have the native library installed. maybe you just need to do pip install lzf?

You mean python-lzf, right?

does the commit @jvtm mentioned changes anything? seem to me that it does the same thing the current version already does. please let me know if i'm missing anything.

You are correct. It seems to do the same check for existence of lzf module.

from redis-rdb-tools.

@billcrook no, not python-lzf that's the python re-implementation.
the fast one, which we rather use is just lzf which are python bindings to the C implementation.

from redis-rdb-tools.

Are you sure about that? When I remove python-lzf and install lzf I get:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<stdin>", line 13, in parse
  File "/Users/billcrook/dev/audiomack/data-pipeline/venv/lib/python3.6/site-packages/rdbtools/parser.py", line 461, in parse_fd
    self.read_object(f, data_type)
  File "/Users/billcrook/dev/audiomack/data-pipeline/venv/lib/python3.6/site-packages/rdbtools/parser.py", line 569, in read_object
    value = self.read_string(f)
  File "/Users/billcrook/dev/audiomack/data-pipeline/venv/lib/python3.6/site-packages/rdbtools/parser.py", line 508, in read_string
    val = self.lzf_decompress(f.read(clen), l)
  File "/Users/billcrook/dev/audiomack/data-pipeline/venv/lib/python3.6/site-packages/rdbtools/parser.py", line 1021, in lzf_decompress
    return lzf.decompress(compressed, expected_length)
AttributeError: module 'lzf' has no attribute 'decompress

from redis-rdb-tools.

For reference: #110 (comment)

from redis-rdb-tools.

@billcrook sorry, it seems that i was wrong.. python-lzf is the one that's native, and redis-rdb-tools has no use of the lzf library.
maybe @galcohen-redislabs can provide some insight or spot a regression.

from redis-rdb-tools.

@billcrook Please provide some rough numbers on the rdb file:
Number of keys, average value size, time it takes to rdb --command json it.

from redis-rdb-tools.