Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use yapic.json instead of json for 1.5x to 2x performance gain #237

Merged
merged 3 commits into from Apr 21, 2020

Conversation

vincentmele
Copy link
Contributor

Documented in #231 .

Tested for a week continuously (on only a couple exchanges, including Huobi) and I see no abnormal results.

@bmoscon
Copy link
Owner

bmoscon commented Apr 14, 2020

you have the wrong dependency here. It should be:

https://pypi.org/project/yapic.json/

So, i dont think you need to re-test for a week, but thats definitely the wrong library. yapic installs tensoflow and a million other reqs that arent needed

@vincentmele
Copy link
Contributor Author

you have the wrong dependency here. It should be:

https://pypi.org/project/yapic.json/

So, i dont think you need to re-test for a week, but thats definitely the wrong library. yapic installs tensoflow and a million other reqs that arent needed

Thanks for catching the error in setup.py. We don't need tensorflow (yet) :-)

@@ -74,7 +74,7 @@ async def _trade(self, msg: dict, timestamp: float):

async def message_handler(self, msg: str, timestamp: float):
# unzip message
msg = zlib.decompress(msg, 16 + zlib.MAX_WBITS)
msg = zlib.decompress(msg, 16 + zlib.MAX_WBITS).decode(encoding='utf-8', errors='strict')
Copy link
Owner

@bmoscon bmoscon Apr 14, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vincentmele looks like the library has been updated to do byte parsing, so you should be able to remove this and require the latest version of the library in setup.py

…te arrays instead of only strings; added minumum requirement of yapic.json 1.41 in setup.py
@bmoscon bmoscon merged commit 11ccbff into bmoscon:master Apr 21, 2020
@dikkechill
Copy link

Good idea to improve the json decoding. How does yapic.json perform compared to e.g. ultrajson?
See https://github.com/ultrajson/ultrajson for some benchmarks with other implementations.

@vincentmele
Copy link
Contributor Author

Good idea to improve the json decoding. How does yapic.json perform compared to e.g. ultrajson?
See https://github.com/ultrajson/ultrajson for some benchmarks with other implementations.

ultrajson doesn't support "parse_float=decimal.Decimal", which is required. I opened the issue here: ultrajson/ultrajson#401, then found that yapic supported it and was extremely fast.

@bmoscon
Copy link
Owner

bmoscon commented Apr 25, 2020

@dikkechill yep - the big requirement here is parsing floats to decimal.Decimal, I haven't found any other libs that support this, but I also haven't exhaustively searched either

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants