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

getMarkets is slow #11

Closed
niran opened this issue Apr 26, 2015 · 14 comments
Closed

getMarkets is slow #11

niran opened this issue Apr 26, 2015 · 14 comments

Comments

@niran
Copy link
Contributor

niran commented Apr 26, 2015

I'm guessing the sequential RPC calls are holding us up. Potential solutions:

  • Find a way to batch RPC calls.
  • Return some sort of stream so each market can be added to the UI as its calls finish.
@tinybike
Copy link
Member

I recommend making your RPC commands asynchronous. augur-dev-client's getMarket call works this way, and it's quite fast (< 1 sec to load the whole UI on my laptop) -- see https://github.com/AugurProject/augur-dev-client/blob/master/augur/js/augur.js#L376 for how this is implemented.

The dev-client uses EthRPC for its contract function calls. My suggestion is to use EthRPC for your calls (like getMarket) that are resource-intensive (assuming this functionality isn't in web3).

@niran
Copy link
Contributor Author

niran commented Apr 30, 2015

If there's no quick and easy way to make the web3 calls async, EthRPC is happening.

@joeykrug
Copy link
Contributor

ethereum/go-ethereum#814
Make a comment there and ask them to merge it. Then any web3 function call that has a callback will automatically be async, no callback & it'll be synchronous.

@niran
Copy link
Contributor Author

niran commented Apr 30, 2015

I think that bug only affects geth's console. The problem here is that web3's contract function calls don't take callbacks. https://github.com/ethereum/web3.js/blob/master/lib/web3/function.js#L77

Its current API design kind of precludes adding callbacks. The optional last parameter is already a transaction object. On the plus side, writing a SolidityFunction.prototype.callAsync would be a pretty easy fix.

@tinybike
Copy link
Member

Interesting. Well, if web3 can support async callbacks on contract function calls, great. If not, why not just use EthRPC? I've already tested it and have verified that it works -- both in general, as well as for this specific function call. (In fact, I've given you the line number that has the exact code you need to call this particular function.) I'm honestly baffled by the lengths you guys are going to avoid this, especially since we're on a very tight timetable.

Include https://github.com/tinybike/ethrpc.js/blob/master/ethrpc.min.js -- and https://github.com/AugurProject/augur-dev-client/blob/master/augur/js/vendor/sha3.min.js if you don't have it already -- in your main HTML file and you're done. Honestly, I'd be happy to add it myself, but there's been so much resistance to using EthRPC I figured you guys would reject it out of hand.

@niran
Copy link
Contributor Author

niran commented Apr 30, 2015

Switching over to a different library just doesn't seem worth it if we can just add our own version of the five line web3 function to make it do what we need to do. I don't know what other issues web3 has caused, but this particular one hasn't taken as much time as it seems. Haven't looked into this problem at all until today.

Side note: I don't know anything about the timetable

@joeykrug
Copy link
Contributor

Main thing is having one cohesive library (web3) that is fully featured. Implementing jacks above would be akin to Chris and I making our serpent and pyeth mods and not submitting pull requests for them.

Other web3 issues have been fixed due to my reporting a bunch of issues (ditto for all the rpc issues, which without those bug reports wouldn't even work at all)

If it can be added in 5 lines Niran, I'd do that, submit a pull request, and comment on that other thread to merge once your PR is in. That'll take like 2 days, we aim to have an alpha in ~10 days is what I've been telling people

And with the above solution we'd still have web3. Eth moves and changes fast, and using our own js wrapper means maintaining it, adding features to it, and having to take our time to fix it when ethereum radically changes things (like say, the entire contract abi system, which is happening btw).

Mixing libraries as a long term solution for this seems like a mess to maintain, as a temporary stopgap it's fine

@tinybike
Copy link
Member

No, because those are just mods to existing software. EthRPC was written from scratch. So it's really just maintaining two different versions of software intended to accomplish the same task, much like what the Ethereum team is doing with its three Ethereum clients.

And anyway, EthRPC isn't a replacement for web3. It's not intended to be. It's a minimal solution to the problem of needing to send asynchronous RPC requests to an Ethereum client from JS, including executing contract functions. web3's a much bigger, fancier package.

@joeykrug
Copy link
Contributor

Sure, except you have referred to it as such in the past:
"we didn't have an alternative, fully functional, fully implemented decentralized smart contract system handy though." --- allegory comparing early eth days to web3 / your wrapper.

Anyway, in open source community I'm a much bigger fan of fixing issues with existing codebases than rolling your own, and if we can add async callbacks in 5 lines, why wouldn't we?

@tinybike
Copy link
Member

It's fully functional in the sense that it does everything which we need from web3. It's not a general-purpose replacement for web3. It can't for example generate hex encoded contract ABI data for Solidity contracts (sure, I could code that in, but I didn't, because our contracts are 100% Serpent and Serpent's data types are simpler).

Look, in my opinion, you have to go with the solution that works. Everything else, including issues like library cohesion, architecture decisions, etc., is less important than getting the client working. This is especially true when a big software package has been unreliable for months and you're hoping that it will be fixed soon. web3 is solving a bigger, more difficult problem than EthRPC solves -- as evidenced by the fact that it took me all of 2 days to write it, and web3's been under construction for months. Fortunately, EthRPC solves our problems.

@joeykrug
Copy link
Contributor

Did you just miss the whole 5 lines comment by Niran above?

I mean the same issue surrounds the UIs. Instead of implementing eth rpc and making a PR to add async calls to this UI, you built your own from scratch.

I don't understand the aversion to collaboration here

Forking your own and starting anew works well for already established projects with dozens of people that end up having a differing vision at some point (see: Linux). We're like 5 people, why aren't we working together?

@niran
Copy link
Contributor Author

niran commented Apr 30, 2015

The slowness was caused by building the JS in debug mode, which includes sourcemaps and other detritus that was apparently slowing things down more than I thought possible.

The web3 async patch is web3/web3.js#190.

@niran niran closed this as completed Apr 30, 2015
@niran
Copy link
Contributor Author

niran commented May 1, 2015

Spoke too soon. It looks like the only fast combination is async with browserify's debug set to false.

@niran niran reopened this May 1, 2015
@niran
Copy link
Contributor Author

niran commented May 5, 2015

EthRPC happened

@niran niran closed this as completed May 5, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants