Xiao-zhen-Liu commented on code in PR #6927:
URL: https://github.com/apache/texera/pull/6927#discussion_r3726514613
##########
frontend/src/app/workspace/service/joint-ui/joint-ui.service.ts:
##########
@@ -491,11 +491,24 @@ export class JointUIService {
* @param isOperatorValid
*/
public changeOperatorColor(jointPaper: joint.dia.Paper, operatorID: string,
isOperatorValid: boolean): void {
- if (isOperatorValid) {
- jointPaper.getModelById(operatorID).attr("rect.body/stroke", "#CFCFCF");
- } else {
- jointPaper.getModelById(operatorID).attr("rect.body/stroke", "red");
+ this.paintOperatorBorder(jointPaper, operatorID, isOperatorValid ?
"#CFCFCF" : "red");
+ }
+
+ /**
+ * Sets the operator's border stroke, returning early when it is already that
+ * color. A same-value attr() write would not re-render (Backbone's Model.set
+ * no-ops via _.isEqual), but attr() still deep-clones and deep-compares the
+ * whole attrs tree before reaching that check (~46us on our elements, down
to
Review Comment:
Optional, and this is my number being over-quoted rather than your mistake.
`~46us` / `~0.6us` / `roughly 99%` came from a synthetic node benchmark I
ran against a reconstructed 25-selector attrs tree, not from a profiler on the
running app. The ratio holds up, but the absolute microseconds will drift with
element shape and JS engine, so a specific figure in a source comment reads
more precise than the evidence behind it.
I'd either drop the numbers and keep the mechanism ("attr() deep-clones and
deep-compares the whole attrs tree before reaching that check, which this
skips"), or keep them and say they're a rough local measurement. Either way,
fine to merge as is.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
