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

Add benchmark dir in tests #334

Closed
smol-ninja opened this issue Apr 28, 2024 · 9 comments
Closed

Add benchmark dir in tests #334

smol-ninja opened this issue Apr 28, 2024 · 9 comments
Assignees
Labels
effort: high Large or difficult task. priority: 1 This is important. It should be dealt with shortly. type: test Adding, updating, or removing tests. work: complicated Sense-analyze-respond. The relationship between cause and effect requires analysis or expertise.

Comments

@smol-ninja
Copy link
Member

Same as sablier-labs/v2-core#908

@smol-ninja smol-ninja self-assigned this Apr 28, 2024
@smol-ninja smol-ninja added effort: high Large or difficult task. type: test Adding, updating, or removing tests. priority: 2 We will do our best to deal with this. work: complicated Sense-analyze-respond. The relationship between cause and effect requires analysis or expertise. labels Apr 28, 2024
@PaulRBerg
Copy link
Member

it would be helpful to provide this, a user just asked about it

SCR-20240621-joyc

@smol-ninja smol-ninja added priority: 1 This is important. It should be dealt with shortly. and removed priority: 2 We will do our best to deal with this. labels Jun 21, 2024
@smol-ninja
Copy link
Member Author

Alright. Marked it as a higher priority

@smol-ninja
Copy link
Member Author

smol-ninja commented Jun 26, 2024

Any suggestions on the benchmark table? I assumed that users might be more interested into understanding gas usage based on batch size rather than number of segments/tranches. The latter is already documented in the v2-core repo.

| Function                 | Lockup Type     | Segments/Tranches | Batch Size | Gas Usage |
| ------------------------ | --------------- | ----------------- | ---------- | --------- |
| `createWithDurationsLL`  | Lockup Linear   |                   | 2          | 361844    |
| `createWithTimestampsLL` | Lockup Linear   |                   | 2          | 324523    |
| `createWithDurationsLD`  | Lockup Dynamic  | 5                 | 2          | 608910    |
| `createWithTimestampsLD` | Lockup Dynamic  | 5                 | 2          | 578586    |
| `createWithDurationsLT`  | Lockup Tranched | 5                 | 2          | 601881    |
| `createWithTimestampsLT` | Lockup Tranched | 5                 | 2          | 572680    |
| `createWithDurationsLL`  | Lockup Linear   |                   | 5          | 734485    |
| `createWithTimestampsLL` | Lockup Linear   |                   | 5          | 732963    |
| `createWithDurationsLD`  | Lockup Dynamic  | 5                 | 5          | 1391832   |
| `createWithTimestampsLD` | Lockup Dynamic  | 5                 | 5          | 1368315   |
| `createWithDurationsLT`  | Lockup Tranched | 5                 | 5          | 1374272   |
| `createWithTimestampsLT` | Lockup Tranched | 5                 | 5          | 1353582   |
| `createWithDurationsLL`  | Lockup Linear   |                   | 10         | 1417161   |
| `createWithTimestampsLL` | Lockup Linear   |                   | 10         | 1414212   |
| `createWithDurationsLD`  | Lockup Dynamic  | 5                 | 10         | 2732969   |
| `createWithTimestampsLD` | Lockup Dynamic  | 5                 | 10         | 2686132   |
| `createWithDurationsLT`  | Lockup Tranched | 5                 | 10         | 2697702   |
| `createWithTimestampsLT` | Lockup Tranched | 5                 | 10         | 2656477   |
| `createWithDurationsLL`  | Lockup Linear   |                   | 20         | 2783678   |
| `createWithTimestampsLL` | Lockup Linear   |                   | 20         | 2778224   |
| `createWithDurationsLD`  | Lockup Dynamic  | 5                 | 20         | 5419557   |
| `createWithTimestampsLD` | Lockup Dynamic  | 5                 | 20         | 5326631   |
| `createWithDurationsLT`  | Lockup Tranched | 5                 | 20         | 5348503   |
| `createWithTimestampsLT` | Lockup Tranched | 5                 | 20         | 5266764   |
| `createWithDurationsLL`  | Lockup Linear   |                   | 50         | 6889015   |
| `createWithTimestampsLL` | Lockup Linear   |                   | 50         | 6877961   |
| `createWithDurationsLD`  | Lockup Dynamic  | 5                 | 50         | 13503395  |
| `createWithTimestampsLD` | Lockup Dynamic  | 5                 | 50         | 13276192  |
| `createWithDurationsLT`  | Lockup Tranched | 5                 | 50         | 13318736  |
| `createWithTimestampsLT` | Lockup Tranched | 5                 | 50         | 13124266  |

cc @sablier-labs/engineers.


Do we also want benchmark tables for MerkleLockupFactory and MerkleLL/LT contracts? IMO that should not be necessary since Airstream campaign creators are usually the companies which have very few incentive to care about transaction cost.

@PaulRBerg
Copy link
Member

Yes, batch size should be more important than the number of segments/ tranches. Feedback:

  • A batch size of 2 isn't helpful. Users can just look at single streams and double the gas cost to estimate it. Let's do a size of 30 or 40 instead.
  • Can we do 24 segments/ tranches instead of 5 (two years' worth of vesting)?

@smol-ninja
Copy link
Member Author

  • Instead of [2,5,10,20,50], we want [30,40,50]?
  • Yes we can do 24 instead of 5 for segments/tranches.

@PaulRBerg
Copy link
Member

Let's do [5,10,20,30,50].

@smol-ninja
Copy link
Member Author

I now remember why I avoided large number of segments with higher batch size. Because it exceeds, block gas limit. With 24 segments, its not feasible to have batch size of more than 30 in LD and LT.

| Function                 | Lockup Type     | Segments/Tranches | Batch Size | Gas Usage |
| ------------------------ | --------------- | ----------------- | ---------- | --------- |
| `createWithDurationsLL`  | Lockup Linear   |                   | 5          | 771013    |
| `createWithTimestampsLL` | Lockup Linear   |                   | 5          | 732772    |
| `createWithDurationsLD`  | Lockup Dynamic  | 24                | 5          | 3951599   |
| `createWithTimestampsLD` | Lockup Dynamic  | 24                | 5          | 3815274   |
| `createWithDurationsLT`  | Lockup Tranched | 24                | 5          | 3862661   |
| `createWithTimestampsLT` | Lockup Tranched | 24                | 5          | 3744535   |
| `createWithDurationsLL`  | Lockup Linear   |                   | 10         | 1417180   |
| `createWithTimestampsLL` | Lockup Linear   |                   | 10         | 1414248   |
| `createWithDurationsLD`  | Lockup Dynamic  | 24                | 10         | 7819162   |
| `createWithTimestampsLD` | Lockup Dynamic  | 24                | 10         | 7585613   |
| `createWithDurationsLT`  | Lockup Tranched | 24                | 10         | 7632113   |
| `createWithTimestampsLT` | Lockup Tranched | 24                | 10         | 7444113   |
| `createWithDurationsLL`  | Lockup Linear   |                   | 20         | 2783509   |
| `createWithTimestampsLL` | Lockup Linear   |                   | 20         | 2779081   |
| `createWithDurationsLD`  | Lockup Dynamic  | 24                | 20         | 15617207  |
| `createWithTimestampsLD` | Lockup Dynamic  | 24                | 20         | 15131248  |
| `createWithDurationsLT`  | Lockup Tranched | 24                | 20         | 15211892  |
| `createWithTimestampsLT` | Lockup Tranched | 24                | 20         | 14846363  |
| `createWithDurationsLL`  | Lockup Linear   |                   | 30         | 4143337   |
| `createWithTimestampsLL` | Lockup Linear   |                   | 30         | 4148585   |
| `createWithDurationsLD`  | Lockup Dynamic  | 24                | 30         | 23460912  |
| `createWithTimestampsLD` | Lockup Dynamic  | 24                | 30         | 22697560  |
| `createWithDurationsLT`  | Lockup Tranched | 24                | 30         | 22794686  |
| `createWithTimestampsLT` | Lockup Tranched | 24                | 30         | 22267335  |
| `createWithDurationsLL`  | Lockup Linear   |                   | 50         | 6871104   |
| `createWithTimestampsLL` | Lockup Linear   |                   | 50         | 6893873   |
| `createWithDurationsLD`  | Lockup Dynamic  | 24                | 50         | 39222140  |
| `createWithTimestampsLD` | Lockup Dynamic  | 24                | 50         | 37860731  |
| `createWithDurationsLT`  | Lockup Tranched | 24                | 50         | 37962234  |
| `createWithTimestampsLT` | Lockup Tranched | 24                | 50         | 37136749  |

@smol-ninja
Copy link
Member Author

How about with batch size of 50, I change segments/tranches to 12 i.e. one year's worth of stream?

@PaulRBerg
Copy link
Member

I change segments/tranches to 12 i.e. one year's worth of stream?

Sounds good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort: high Large or difficult task. priority: 1 This is important. It should be dealt with shortly. type: test Adding, updating, or removing tests. work: complicated Sense-analyze-respond. The relationship between cause and effect requires analysis or expertise.
Projects
None yet
Development

No branches or pull requests

2 participants