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

Optimize registry #66

Open
raamb opened this issue Nov 1, 2018 · 14 comments
Open

Optimize registry #66

raamb opened this issue Nov 1, 2018 · 14 comments
Assignees

Comments

@raamb
Copy link
Member

raamb commented Nov 1, 2018

Attempt to optimize registry gas costs

@astroseger
Copy link
Collaborator

astroseger commented Nov 25, 2018

The reason why registry is so expensive is the function "listServicesForTag", which returns list of services for the specific tag.

This functionality is the reason why adding one tag to the service costs ~250.000 gas and not ~20.000-40.000, and removing a Service or a Organization costs unspeakable amount of gas.

Let's discuss do we really need this function. First, I want to dispel the main misconception, about possible usefulness of this function for on-chain calls.

This function will never be used for on-chain "search" for services, simply because it gives only list of services for one tag. But in any realistic scenarios we want to get list of services which have several tag (for example "classify" and "cats", or "classify" and "video" and "faces"). Only imagine how much it will costs to calculate intersection between several unsorted lists inside on-chain call!

Conclusion: this function will never be used for on-chain "discovery" of services.

It means that, the only one possible application of this function is "off-chain" search (in dApp for example) for services which has specific tags.

The main remaining question is following: do we really need on-chain support for this functionality?

In order to answer this question we need to more precisely define user cases, which I will do in separate issue.

@ksridharbabuus
Copy link
Contributor

@astroseger As per our discussions in HK, this functionality is to identify the service for a given tag within onChian. We even debated saying whether we need this functionality as we are having a separate structure to manage this and also for CRUD operations takes longer time on this array. As per the requirements we should be able to get the list of services for a given tag within onChain, in order to satisfy this requirement we need this piece of code.
Please note that in DApp we are going to use cache service to list the services for a given tag.

@astroseger
Copy link
Collaborator

@ksridharbabuus I only said that this function will not be used from another contracts.

@astroseger
Copy link
Collaborator

@ksridharbabuus Actually I haven't understood you comment.. If in DApp you use the cache service for tag search, so it means you don't need this function in the Registry for dApp... Is it correct?

@astroseger
Copy link
Collaborator

astroseger commented Nov 25, 2018

Just to make it clear... I haven't proposed to remove this functionality (listServicesForTag). I only stated that

  • it is very expensive in term of gas to maintain this functionality (if it is expensive it doesn't mean it is useless).
  • We will never use this functionality for making discovery of services from another contracts, simply because it would be impossible in terms of gas.

@ksridharbabuus
Copy link
Contributor

@astroseger Yes for DApp definitely we are not going to use this function as we are caching based on the events.
As per our discussion in HK that we might need to this to support onChian needs especially for decentralized requirements and to get the service details from another contract based on tag. Otherwise we dont have any other option to get these details except the event listening.

@ksridharbabuus
Copy link
Contributor

@astroseger listServicesForTag is a view function and should not consume any gas. Might be little higher during the deployment.
Major gas consumed in managing & operating on this list.

@astroseger
Copy link
Collaborator

astroseger commented Nov 25, 2018

@ksridharbabuus I'm talking about calling this function from another contracts!!!

  1. Calling view function from another contract costs gas
  2. I want to repeat it again. We don't need simply list of services for one tag. What we need is intersection between these lists. For example we could need services which have tag "dog" and tag "classify"... Please estimate how much gas it would costs to calculate intersection between two unordered lists in the contact!

@ksridharbabuus
Copy link
Contributor

@astroseger unfortunately we should not get search functionality on to the smart contract, it can become complex as we go forward. Not able to convince everyone during the HK discussion on this topic. If the requirements say we dont need then we can remove this functionality itself not only the function. I am in favor of doing this functionality offchain.

@astroseger
Copy link
Collaborator

astroseger commented Nov 25, 2018

What we proved for now is that one of argument in favor of this functionality (possibility of calling it from another smart contracts ) which we've discussed in HK was wrong.

@ksridharbabuus
Copy link
Contributor

Definitely it will. I can create a contract to demonstrate the same.
Please note that without function like this we cannot expose the list of services for a given tag.
So InShort, Tag Search can be removed.

@ferrouswheel
Copy link

I would argue that searching for services by tag on chain is not necessary. Building an off-chain index from ipfs metadata makes more sense to me. We could even provide official snet tooling to build/maintain such an index locally, and update by listening to events.

This local index based on metadata would also be far more usable. You could e.g. search for services that conform to a particular grpc API, which I think would be more useful for the ultimate goal of SingularityNET (automatic service discovery between services).

@ksridharbabuus
Copy link
Contributor

@ferrouswheel Completely agree. If needed we can provide similar to SPARQL query language for search operations on the client side (CLI/SDK/DApp[Cache]).

@raamb
Copy link
Member Author

raamb commented Dec 6, 2018

Refer to #76 (comment)
We revisit post beta if required.

@raamb raamb removed the beta label Jan 9, 2019
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

No branches or pull requests

4 participants