refactor(core): use version>0 instead of hasRun#62467
refactor(core): use version>0 instead of hasRun#62467milomg wants to merge 1 commit intoangular:mainfrom
Conversation
| return; | ||
| } | ||
| this.hasRun = true; | ||
| this.version++; |
There was a problem hiding this comment.
I guess setting a value to true will be cheaper memory wise, as there would be no need to allocate memory for each new value that ++ generates, while the boolean flag only allocates memory once, and uses that memory location.
There was a problem hiding this comment.
It's just a number though (fixed 64 bit), It shouldn't really matter ?
There was a problem hiding this comment.
This has to go through the perf benchmarks I guess.
There was a problem hiding this comment.
In fact, in V8 this is going to be stored as an SMI (so a 32 bit integer). So unless there will be 2 billion increments, the impact is pretty minimal.
this saves a field for effect and watch nodes
|
TESTED=TGP |
thePunderWoman
left a comment
There was a problem hiding this comment.
LGTM
reviewed-for: fw-general, public-api, primitives
|
TGP is green. Safe to merge |
|
I see that you just added the If you want your PR to be merged, it has to pass all the CI checks. If you can't get the PR to a green state due to flakes or broken |
|
LGTM reviewed-for: primitives-shared |
this saves a field for effect and watch nodes PR Close #62467
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: #63667
What is the new behavior?
This saves a field for effect and watch nodes. Additionally, it should make effect nodes flash in the devtools when they rerun.
Does this PR introduce a breaking change?
Other information