Conversation
shawntabrizi
left a comment
shawntabrizi
left a comment
There was a problem hiding this comment.
Should be run one more time?
Was there issue having the bechmarking machine hooked up to this repo?
Yup we def should right before launch, also I currently have a PR out that will change the runtime #88 that would also require a re run |
apopiak
left a comment
apopiak
left a comment
There was a problem hiding this comment.
LGTM, just one suspicious change
| fn new_session(_r: u32, c: u32, ) -> Weight { | ||
| (37_164_000 as Weight) | ||
| // Standard Error: 5_000 | ||
| .saturating_add((5_724_000 as Weight).saturating_mul(c as Weight)) |
There was a problem hiding this comment.
that's a quite significant weight change
wanna comment on that?
There was a problem hiding this comment.
the logic of this function must have changed (hence the additional read per C below), so dunno what level of explanation you are looking for.
There was a problem hiding this comment.
I also thought this was weird so ended up realizing that the removal logic was slightly changed and I was missing that block, tested this locally, fixed it, running it now on the benchmark machine again
There was a problem hiding this comment.
hmm same results on the benchmark machine (may just be in my head), going to look into this more (gonna have to run the benchmarks again later) for now will merge.
There was a problem hiding this comment.
@shawntabrizi
I just thought it deserved extra attention and scrutiny 🤷 😄
3 participants
These weights were done on the benchmark machine. They will most likely need to be redone before launch so it is tied to #37 but maybe we should leave it open just to make sure we re run it based on the finalized chain