Introduce performance optimizations and new INP metric #436
Conversation
This introduction enable users to use already decoded TCModel to enhance performances
| // this will enable you to skip the TCString.decode call | ||
| // decodeWithCache if set to true it will call the TCString.decode using the previously TCModel decoded in cache | ||
| // this skip a lot of works | ||
| cmpApi.update(encodedTCString, false, {preDecodedTCModel: null, decodeWithCache: false}); |
There was a problem hiding this comment.
I am not sure we can change api without discussion and approval, also these parameters are looks very complicated to manage, is it possible to make this improvement always running or it does not make sense for all cases?
There was a problem hiding this comment.
The purpose of this additional parameter is not to introduce breaking changes, since in any case the first two parameters that were already used do not change. Honestly, I was careful not to introduce breaking changes to avoid behaviors that those who already use the library might not expect, but we can also evaluate other solutions. Honestly, I haven't introduced mechanisms or classes to avoid adding weight and complexity to the project, but I don't think it's a problem
|
We have implemented this maybe we can reuse some ideas here |
Are you saying to do a similar thing for all the places where I applied memoization? |
I like this implementation a lot better. It does the caching without the need to change any APIs, which is much preferred. |
|
There is also this open PR suggesting improvements to the performance to the pub restrictions code: #424 . |
So you say to apply the advice suggested by @sevriugin? Tell me if you want me to apply the solution suggested by @sevriugin and I will update the PR, otherwise if you have other suggestions we can think about it |
|
Can you review? |
|
We have testes this one didomi#4 for more then one year so it will be save to apply and no action needed from library users to use this feature, imo, it's better do not change API if we can avoid it, hope it makes sense |
I will apply the same changes considering that in this PR there are additional improvements in this PR, thanks for the advice 🙏 I will update you asap |
closes #435
The goal of this PR is to introduce a way to allow users of this library to make use of some optimizations that can be made.
This will help us improve INP metrics since the nature of a CMP is very much linked to the new metric, it would also allow us to ensure a better user experience overall.
Here the article where INP is explained https://web.dev/articles/inp
Just let me know if you need some screenshots.
@sevriugin Hope in a review soon, thanks in advance