Skip to content

Conversation

@welshjf
Copy link
Contributor

@welshjf welshjf commented Jul 8, 2015

Switching to simplejson seems to improve CPU and memory use, particularly for the expensive getrawmempool(True).

I also tried ujson but it doesn't support the parse_float parameter.

bitcoin/rpc.py Outdated
Copy link
Owner

Choose a reason for hiding this comment

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

Make "By default" on a separate line, with a space between it and the title "Bitcoin Core RPC support"

@petertodd
Copy link
Owner

Seems like a reasonable idea; simplejson is supported on both python3 and python2.

It'd be better though if this could be an opt-in - any ideas on making that possible? If there's a json bug people could of course lose money here...

@welshjf
Copy link
Contributor Author

welshjf commented Jul 9, 2015

Opt-in was indeed my intention, but on a fresh look, my approach is perhaps overly magical. Explicit is better than implicit, as they say in Python. If the user wants to use two different json libs at once, it's not for us to say "no".

The difficulty is, that it's nice to have everything imported early at global scope, such as for ease of reference, and static checkers.

Perhaps initially always import json, then have a function to replace it with something else. An added benefit is this will support any (API-compatible) json lib, at the user's risk.

@welshjf
Copy link
Contributor Author

welshjf commented Jul 9, 2015

In fact, this is already possible with no new code:

import bitcoin.rpc
import simplejson
bitcoin.rpc.json = simplejson

Can't say I exactly like it, but haven't got anything better. Does this seem reasonable to document as a supported thing?

@petertodd
Copy link
Owner

Sure, let's do that.

On 9 July 2015 14:09:24 GMT-04:00, Jacob Welsh notifications@github.com wrote:

In fact, this is already possible with no new code:

import bitcoin.rpc
import simplejson
bitcoin.rpc.json = simplejson

Can't say I exactly like it, but haven't got anything better. Does this
seem reasonable to document as a supported thing?


Reply to this email directly or view it on GitHub:
#74 (comment)

@welshjf
Copy link
Contributor Author

welshjf commented Jul 10, 2015

Updated.

@petertodd petertodd merged commit ff4b914 into petertodd:master Jul 17, 2015
petertodd added a commit that referenced this pull request Jul 17, 2015
ff4b914 rpc: document using an alternate JSON module (Jacob Welsh)
ghtdak pushed a commit to ghtdak/python-bitcoinlib that referenced this pull request Dec 1, 2015
ff4b914 rpc: document using an alternate JSON module (Jacob Welsh)

 [ yapified by gitreformat (github/ghtdak) on Mon Nov 30 21:12:34 2015 ]
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.

2 participants