Skip to content

perf: Workflow Canvas Rendering #2250

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Feb 12, 2025
Merged

perf: Workflow Canvas Rendering #2250

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 3 additions & 48 deletions ui/src/workflow/common/NodeCascader.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,51 +49,12 @@ const wheel = (e: any) => {

function visibleChange(bool: boolean) {
if (bool) {
options.value = getIncomingNode(props.nodeModel.id)
options.value = props.nodeModel.get_up_node_field_list(false, true)
}
}

function _getIncomingNode(id: String, startId: String, value: Array<any>) {
let list = props.nodeModel.graphModel.getNodeIncomingNode(id)
list = list.filter((item: any) => item.id !== startId)
let firstElement = null
if (list.length > 0) {
list.forEach((item: any) => {
if (!value.some((obj: any) => obj.id === item.id)) {
if (!value.some((value_item) => value_item.value === item.id)) {
value.unshift({
value: item.id,
label: item.properties.stepName,
type: item.type,
children: item.properties?.config?.fields || []
})
if (item.properties?.globalFields && item.type === 'start-node') {
firstElement = {
value: 'global',
label: t('views.applicationWorkflow.variable.global'),
type: 'global',
children: item.properties?.config?.globalFields || []
}
}
}
}
})

list.forEach((item: any) => {
_getIncomingNode(item.id, startId, value)
})
}
if (firstElement) {
value.unshift(firstElement)
}
return value
}
function getIncomingNode(id: string) {
return _getIncomingNode(id, id, [])
}
const validate = () => {
const incomingNodeValue = getIncomingNode(props.nodeModel.id)
options.value = incomingNodeValue
const incomingNodeValue = props.nodeModel.get_up_node_field_list(false, true)
if (!data.value || data.value.length === 0) {
return Promise.reject(t('views.applicationWorkflow.variable.ReferencingRequired'))
}
Expand All @@ -113,15 +74,9 @@ const validate = () => {
}
return Promise.resolve('')
}
props.nodeModel.graphModel.eventCenter.on('refresh_incoming_node_field', () => {
options.value = getIncomingNode(props.nodeModel.id)
})
props.nodeModel.graphModel.eventCenter.on('refreshFileUploadConfig', () => {
options.value = getIncomingNode(props.nodeModel.id)
})
defineExpose({ validate })
onMounted(() => {
options.value = getIncomingNode(props.nodeModel.id)
options.value = props.nodeModel.get_up_node_field_list(false, true)
})
</script>
<style scoped></style>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code appears to be handling node field changes and validation within an application workflow. It includes fetching incoming nodes, validating the current selection of fields, and handling events related to refreshing node fields.

Here are some optimizations and improvements suggested:

  1. Avoiding Deep Copies: The _getIncomingNode function modifies the value array in place rather than cloning it before modifications. This approach reduces overhead but may affect performance if used improperly. Ensure that modifying arrays does not have side effects elsewhere in your application.

  2. TypeScript Annotations: Adding TypeScript annotations for type safety can help catch errors early during development. For example:

    declare const t: (...args: any[]) => string;
interface NodeModel {
    graphModel: { getNodeIncomingNode: (id: string) => any[] };
    id?: string;
    // Add more properties as needed
}

  3. Refactoring Event Listeners: Consider refactoring event listeners outside of the main logic to improve maintainability and reduce complexity. You could create separate functions or classes for managing these event subscriptions.

  4. Comments and Documentation: Add comments explaining complex logic or assumptions for clarity. Also consider adding documentation comments or a README.md file for better understanding of the code's purpose and functionality.

  5. Testing: Implement unit tests for specific parts of the codebase, especially those involving state management and data fetching.

  6. Error Handling: Enhance error handling by adding more detailed messages returned from promises or logging exceptions for easier debugging.

By addressing these points, you can make the code cleaner, more efficient, and potentially easier to maintain and extend in the future.

69 changes: 65 additions & 4 deletions ui/src/workflow/common/app-node.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,25 @@ import i18n from '@/locales'
import { WorkflowType } from '@/enums/workflow'
import { nodeDict } from '@/workflow/common/data'
import { isActive, connect, disconnect } from './teleport'
import { t } from '@/locales'
import { type Dict } from '@/api/type/common'
class AppNode extends HtmlResize.view {
isMounted
r?: any
component: any
app: any
root?: any
VueNode: any
up_node_field_dict?: Dict<Array<any>>
constructor(props: any, VueNode: any) {
super(props)
this.component = VueNode
this.isMounted = false
props.model.clear_next_node_field = this.clear_next_node_field.bind(this)
props.model.get_up_node_field_dict = this.get_up_node_field_dict.bind(this)
props.model.get_node_field_list = this.get_node_field_list.bind(this)
props.model.get_up_node_field_list = this.get_up_node_field_list.bind(this)

if (props.model.properties.noRender) {
delete props.model.properties.noRender
} else {
Expand All @@ -30,21 +38,74 @@ class AppNode extends HtmlResize.view {
}
}
function getNodesName(num: number) {
let number = num
const number = num
const name = props.model.properties.stepName + number
if (!props.graphModel.nodes?.some((node: any) => node.properties.stepName === name.trim())) {
props.model.properties.stepName = name
} else {
number += 1
getNodesName(number)
getNodesName(number + 1)
}
}
props.model.properties.config = nodeDict[props.model.type].properties.config
if (props.model.properties.height) {
props.model.height = props.model.properties.height
}
}
get_node_field_list() {
const result = []
if (this.props.model.type === 'start-node') {
result.push({
value: 'global',
label: t('views.applicationWorkflow.variable.global'),
type: 'global',
children: this.props.model.properties?.config?.globalFields || []
})
}
result.push({
value: this.props.model.id,
label: this.props.model.properties.stepName,
type: this.props.model.type,
children: this.props.model.properties?.config?.fields || []
})
return result
}
get_up_node_field_dict(contain_self: boolean, use_cache: boolean) {
if (!this.up_node_field_dict || !use_cache) {
const up_node_list = this.props.graphModel.getNodeIncomingNode(this.props.model.id)
this.up_node_field_dict = up_node_list
.filter((node) => node.id != 'start-node')
.map((node) => node.get_up_node_field_dict(true, use_cache))
.reduce((pre, next) => ({ ...pre, ...next }), {})
}
if (contain_self) {
return {
...this.up_node_field_dict,
[this.props.model.id]: this.get_node_field_list()
}
}
return this.up_node_field_dict ? this.up_node_field_dict : {}
}

get_up_node_field_list(contain_self: boolean, use_cache: boolean) {
const result = Object.values(this.get_up_node_field_dict(contain_self, use_cache)).reduce(
(pre, next) => [...pre, ...next],
[]
)
const start_node_field_list = this.props.graphModel
.getNodeModelById('start-node')
.get_node_field_list()
return [...start_node_field_list, ...result]
}

clear_next_node_field(contain_self: boolean) {
const next_node_list = this.props.graphModel.getNodeOutgoingNode(this.props.model.id)
next_node_list.forEach((node) => {
node.clear_next_node_field(true)
})
if (contain_self) {
this.up_node_field_dict = undefined
}
}
getAnchorShape(anchorData: any) {
const { x, y, type } = anchorData
let isConnect = false
Expand Down Expand Up @@ -276,7 +337,7 @@ class AppNodeModel extends HtmlResize.model {
}

setAttributes() {
const { t } = i18n.global;
const { t } = i18n.global
this.width = this.get_width()
const isLoop = (node_id: string, target_node_id: string) => {
const up_node_list = this.graphModel.getNodeIncomingNode(node_id)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are several areas where improvements can be made to the provided code:

  1. Consistent Method Bindings:

    • The bindings for methods like clear_next_node_field, get_up_node_field_dict, and get_up_node_field_list should only appear once in the constructor.

  2. Redundant Imports:

    • Remove the duplicate import of connect from ./teleport as it's already imported at the beginning.

  3. Nullish Coalescing Operator:

    • Replace || [] with ?? [] in get_node_field_list to handle cases where config?.fields might be null or undefined more gracefully.

  4. Cache Management:

    • Consider adding caching strategies or memoization to improve performance, especially when the method is called frequently within loops or recursive calculations.

  5. Optimize Function Calls:

    • Avoid unnecessary function calls that could lead to performance bottlenecks, such as re-retrieving properties from props.model.

  6. Variable Naming Consistency:

    • Ensure variable names follow consistent naming conventions for readability and maintainability.

Here's an optimized version based on these suggestions:

@@ -10,9 +10,14 @@
 import i18n from '@/locales';
 import { WorkflowType } from '@/enums/workflow'
 import { nodeDict } from '@/workflow/common/data';
+import { t } from '@/locales';

class AppNode extends HtmlResize.view {
  isMounted: boolean;
  r?;
  component: any;
  app: any;
  root?: HTMLDivElement;
  VueNode: any;

  constructor(props: any, VueNode: any) {
    super(props);
    this.component = VueNode;
    this.isMounted = false;
    
    // Single binding statement for all event handlers
    props.model.events = {
      clearNextNodeField: this.clear_next_node_field.bind(this),
      getUpNodeFieldDict: this.get_up_node_field_dict.bind(this),
      getNodeFieldList: this.getNodeFieldList.bind(this)
    };
    
    if (props.model.properties.noRender) {
      delete props.model.properties.noRender;
    }

    function getNodesName(num: number): string {
      const name = `${props.model.properties.stepName}-${num}`;
      if (!props.graphModel.nodes.some((node: any) => node.properties.stepName === name)) {
        props.model.properties.stepName = name;
      } else {
        return getNodesName(++num);
      }
    }

    props.model.properties.config = nodeDict[props.model.type].properties.config;
    if (props.model.properties.height) {
      props.model.height = props.model.properties.height;
    }
  }

  getNodeFieldList(): Array<{ value: string; label: string; type: string; children?: Array<any> }> {
    const result: Array<{ value: string; label: string; type: string; children?: Array<any> }> = [];
    
    if (this.props.model.type === 'start-node') {
      result.push({
        value: 'global',
        label: t('views.applicationWorkflow.variable.global'),
        type: 'global',
        children: this.props.model.properties?.config?.globalFields ?? []
      });
    }

    result.push({
      value: this.props.model.id,
      label: this.props.model.properties.stepName,
      type: this.props.model.type,
      children: this.props.model.properties?.config?.fields ?? []
    });

    return result;
  }

  getUpNodeFieldDict(containSelf: boolean, useCache: boolean): Record<string, Array<{ value: string; label: string; type: string; children?: Array<any> }>> | undefined {
    let upNodeList = this.props.graphModel.getNodeIncomingNode(this.props.model.id);

    if (!upNodeList.length || upNodeList.every(n => n.id !== 'start-node')) {
      return undefined;
    }

    let upNodeFieldDict: Record<string, Array<{ value: string; label: string; type: string; children?: Array<any> }>> = {};
    for (const node of upNodeList.filter(node => node.id !== 'start-node')) {
      const fields = node.getUpNodeFieldDict(true, useCache)?.[node.id];
      upNodeFieldDict[node.id] = fields;
    }

    if (containSelf) {
      upNodeFieldDict[this.props.model.id] = this.getNodeFieldList();
    }

    return upNodeFieldDict;
  }

  getUpNodeFieldList(containSelf: boolean, useCache: boolean): Array<{ value: string; label: string; type: string; children?: Array<any> }> {
    const baseResult = Object.values(this.getUpNodeFieldDict(containSelf, useCache));
    
    if (baseResult.length > 0) {
      const startNodeFieldList = this.props.graphModel.getNodeModelById('start-node').get_nodeFieldList();
      return [...startNodeFieldList, ...baseResult.flat()];
    }

    return [];
  }

  clearNextNodeField(containSelf: boolean) {
    const nextNodeList = this.props.graphModel.getNodeOutgoingNode(this.props.model.id).filter(node => node.type !== WorkflowType.StartNode);
    nextNodeList.forEach(node => {
      node.clearNextNodeField(true);
    });

    if (containSelf) {
      this.upNodeFieldDict = undefined;
    }
  }
}

These changes ensure better organization and consistency across the codebase while improving readability and potentially enhancing performance through less redundant operations.

Expand Down
1 change: 0 additions & 1 deletion ui/src/workflow/common/edge.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ class CustomEdge2 extends BezierEdge {
super.componentWillUnmount()
}
if (isActive()) {
console.log('unmount')
disconnect(this.targetId())
}
this.unmountVueComponent()
Expand Down
4 changes: 4 additions & 0 deletions ui/src/workflow/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,10 @@ const renderGraphData = (data?: any) => {
lf.value.deleteEdge(id)
})
})
lf.value.graphModel.eventCenter.on('anchor:drop', (data: any) => {
// 清除当前节点下面的子节点的所有缓存
data.nodeModel.clear_next_node_field(false)
})

setTimeout(() => {
lf.value?.fitView()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The provided code appears to be setting up event listeners for changes in a graph layout library (lf) and adjusting the view based on certain events. Here's a brief review and suggested improvements:

Review Points:

  1. EventListener: The line lf.value.graphModel.eventCenter.on('anchor:drop', (data: any) => { correctly adds an event listener for an 'anchor drop' event. It seems intended to handle situations where nodes are dropped onto anchors.

  2. Node Clearing Logic: The .clear_next_node_field(false) method call within the 'anchor:drop' handler is somewhat unclear due to missing context about next_node_field. However, it appears to clear some fields associated with nodes that might have moved as part of anchoring behavior.

  3. Timer: The statement setTimeout(() => { lf.value?.fitView(); }, 0); ensures that fitView is called asynchronously after rendering has finished. This is generally good practice to prevent blocking UI updates during long-running operations.

Optimization Suggestions

  1. Consistent Error Handling: Ensure there's consistent error handling when working with asynchronous actions such as clearing node fields or fitting views. For instance, you could wrap these calls in try-catch blocks to catch potential errors gracefully.

  2. Event Listener Cleanup: If this logic needs to be removed or modified later, consider implementing a way to cleanly remove the event listener using off(), especially if multiple components require similar functionality. This prevents memory leaks over time.

  3. Type Safety: Verify that the types passed to on() and other methods correspond with those expected by the library. Type annotations can help catch type-related issues at compile-time.

  4. Performance Considerations: While not directly addressed in this snippet, ensure that clearing nodes and fitting views do not involve expensive computations or data manipulations.

Overall, the code looks functional for managing interactions and updating the graphical representation based on user input. Minor adjustments like adding clear documentation around unclear parts of the logic and ensuring robust error handling would enhance its maintenance and future reliability.

Expand Down