Performance update for resolveFieldData

[MoMack20]

Object.keys can get expensive when called a lot of times. Adding a try catch for the quickest and probably highest use case can lead to big performance gains. Related to #4296 but for overall performance

I don't have the time to run through the other scenarios, but I think just doing try catch for each scenario would be faster than the call to Object.keys and isFunction. It would look something like.

let value = null;
try {
	value = data[field];
	if (value)
		return value;
}
catch { }

try {
	value = field(data);
	if (value)
		return value;
}
catch { }

try {
	let fields = field.split('.');
	let nestedValue = data;
	for (var i = 0, len = fields.length; i < len; ++i) {
		if (nestedValue == null) {
			return null;
		}
		nestedValue = nestedValue[fields[i]];
	}
	
	return nestedValue;
}
catch { }

return null;

A screen showing the difference when using chrome tools performance tab

I imagine there are other places in the code that could also benefit from this type of improvement.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment

Name	Status	Preview	Comments	Updated (UTC)
primevue	⬜️ Ignored (Inspect)	Visit Preview		Aug 18, 2023 0:17am

[tugcekucukoglu]

Could you create a separate issue for this PR?

[MoMack20]

Could you create a separate issue for this PR?

#4296 Created

[melloware]

Nice @MoMack20 keep em coming! I will make this change in PrimeReact. I agree with this change.

[melloware]

@MoMack20 here is my final version in PrimeReact which short circuits the field or data being undefined.

    static resolveFieldData(data, field) {
        if (!data || !field) {
            // short circuit if there is nothing to resolve
            return null;
        }

        try {
            const value = data[field];

            if (value) return value;
        } catch {
            // Performance optimization: https://github.com/primefaces/primereact/issues/4797
            // do nothing and continue to other methods to resolve field data
        }

        if (Object.keys(data).length) {
            if (this.isFunction(field)) {
                return field(data);
            } else if (ObjectUtils.isNotEmpty(data[field])) {
                return data[field];
            } else if (field.indexOf('.') === -1) {
                return data[field];
            } else {
                let fields = field.split('.');
                let value = data;

                for (var i = 0, len = fields.length; i < len; ++i) {
                    if (value == null) {
                        return null;
                    }

                    value = value[fields[i]];
                }

                return value;
            }
        }

        return null;
    }

[MoMack20]

@melloware let me know if you measure the actual gainz from this. I'm interested to see the real impact.

[melloware]

@MoMack20 its minimal but it makes the code much more clean and readable. It prevents the JS engine from try catching the null pointer on data if data = null and field = "hello". But mostly for clean readability of "short circuiting".

[mertsincan]

Thanks a lot for your contribution!