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

[WIP] Use references instead of values in CallBuilder API #970

Closed
wants to merge 1 commit into from

Conversation

Robbepop
Copy link
Collaborator

@Robbepop Robbepop commented Oct 20, 2021

  • CallBuilder API
  • CreateBuilder API

Seen improvements so far with the Delegator example contract:

Contract Before PR After PR Reason
Accumulator 8.8 kB 8.8 kB Expected since it does not use the CallBuilder API
Adder 13.0 kB 12.9 kB Just minor improvement, need to find out why not more.
Subber 13.0 kB 12.9 kB Same as with Adder.
Delegator 10.8 kB 10.5 kB Minor improvements in WIP state. Expected to improve further.

Note, that the numbers do not yet include the full change so they are WIP.
Also I am sure we can do better in some places.

A problem will be how this will work out after #665 has been merged since the ink! codegen will be changed significantly through it and won't allow as easily to be adjusted than with the current codegen (which is unfortunate in this case, however, in general the new codegen is far superior).

@HCastano
Copy link
Contributor

HCastano commented Feb 4, 2022

This is pretty stale (it was opened before the new codegen was merged), gonna close it

@HCastano HCastano closed this Feb 4, 2022
@xgreenx
Copy link
Collaborator

xgreenx commented Feb 4, 2022

But that change still should be implemented because it will decrease significantly the size of contracts and performance with cross-contact calls

@HCastano
Copy link
Contributor

HCastano commented Feb 4, 2022

It should, but right now there's nobody to implement it. It's still being tracked in #949, so we're not totally forgetting about it

@Robbepop
Copy link
Collaborator Author

Robbepop commented Feb 4, 2022

To give a little bit of background why this staled: The implementation of this experimental PR just got waaaay too ugly which is never a good sign to further follow a road. So I stopped with this road and thought about another way of solving this instead. However, as it happens there were more important tasks to do at that time.
The wins with respect to file sizes were also pretty minor imo compared to the introduced ugliness and diff.

@HCastano HCastano deleted the robin-use-references-in-call-builder branch July 29, 2022 17:01
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.

3 participants