Skip to content
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.

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

Performance issues with deepMap #3266

Open
wants to merge 22 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
7b10e01
Faster deepMap recurse
dvd101x Sep 16, 2024
c7d98df
Callback with right number of arguments.
dvd101x Sep 16, 2024
8324f63
Removed return in forEach
dvd101x Sep 16, 2024
0aad351
Merge branch 'develop' into deepMap-2
josdejong Sep 18, 2024
9c42720
Clone with slice
dvd101x Sep 19, 2024
2ec54ad
Added index benchmarks
dvd101x Sep 19, 2024
81f6d76
Format
dvd101x Sep 20, 2024
edc61eb
Optimize callback returns the number of arguments
dvd101x Sep 20, 2024
9f80b90
Added tests for callbacks with three arguments.
dvd101x Sep 20, 2024
a5618b5
optimizeCallback returns a non typed function.
dvd101x Sep 21, 2024
c7a4210
Optimize recursre functions by mapping the last dimension.
dvd101x Oct 1, 2024
488628c
Merge branch 'develop' into deepMap-2
dvd101x Dec 6, 2024
3a44e32
Merge branch 'develop' into deepMap-2
dvd101x Dec 16, 2024
f98a164
Updated tests
dvd101x Dec 16, 2024
e59624a
Number of arguments in functions
dvd101x Feb 3, 2025
a6e0526
feat: add findFirst utility and enhance array mapping functions
dvd101x Feb 3, 2025
a0e7ca5
fix deepForEach and enhance forEach utility for better readability
dvd101x Feb 3, 2025
4a507e3
refactor deepMap to simplify zero-skipping logic and utilize iterableMap
dvd101x Feb 3, 2025
5eaa11e
fix: add input validation to map and forEach functions
dvd101x Feb 4, 2025
aee64f7
refactor: simplify recursion logic in map and forEach functions
dvd101x Feb 5, 2025
66cdc32
fix: add handling for empty matrix and array cases in findFirstValueA…
dvd101x Feb 5, 2025
832e91e
fix: improve findFirstValueAndIndex function
dvd101x Feb 5, 2025
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
4 changes: 2 additions & 2 deletions src/function/matrix/forEach.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { optimizeCallback } from '../../utils/optimizeCallback.js'
import { factory } from '../../utils/factory.js'
import { recurse } from '../../utils/array.js'
import { deepForEach } from '../../utils/array.js'

const name = 'forEach'
const dependencies = ['typed']
Expand Down Expand Up @@ -45,5 +45,5 @@ export const createForEach = /* #__PURE__ */ factory(name, dependencies, ({ type
* @private
*/
function _forEach (array, callback) {
recurse(array, [], array, optimizeCallback(callback, array, name))
deepForEach(array, array, optimizeCallback(callback, array, name))
}
4 changes: 2 additions & 2 deletions src/function/matrix/map.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { optimizeCallback } from '../../utils/optimizeCallback.js'
import { arraySize, broadcastSizes, broadcastTo, get, recurse } from '../../utils/array.js'
import { arraySize, broadcastSizes, broadcastTo, get, deepMap } from '../../utils/array.js'
import { factory } from '../../utils/factory.js'

const name = 'map'
Expand Down Expand Up @@ -151,6 +151,6 @@ export const createMap = /* #__PURE__ */ factory(name, dependencies, ({ typed })
* @private
*/
function _mapArray (array, callback) {
return recurse(array, [], array, optimizeCallback(callback, array, name))
return deepMap(array, array, optimizeCallback(callback, array, name))
}
})
6 changes: 3 additions & 3 deletions src/type/matrix/DenseMatrix.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { isArray, isBigNumber, isCollection, isIndex, isMatrix, isNumber, isString, typeOf } from '../../utils/is.js'
import { arraySize, getArrayDataType, processSizesWildcard, reshape, resize, unsqueeze, validate, validateIndex, broadcastTo, get, recurse } from '../../utils/array.js'
import { arraySize, getArrayDataType, processSizesWildcard, reshape, resize, unsqueeze, validate, validateIndex, broadcastTo, get, deepMap, deepForEach } from '../../utils/array.js'
import { format } from '../../utils/string.js'
import { isInteger } from '../../utils/number.js'
import { clone, deepStrictEqual } from '../../utils/object.js'
Expand Down Expand Up @@ -541,7 +541,7 @@ export const createDenseMatrixClass = /* #__PURE__ */ factory(name, dependencies

// determine the new datatype when the original matrix has datatype defined
// TODO: should be done in matrix constructor instead
const data = recurse(me._data, [], me, fastCallback)
const data = deepMap(me._data, me, fastCallback)
const datatype = me._datatype !== undefined
? getArrayDataType(data, typeOf)
: undefined
Expand All @@ -559,7 +559,7 @@ export const createDenseMatrixClass = /* #__PURE__ */ factory(name, dependencies
// matrix instance
const me = this
const fastCallback = optimizeCallback(callback, me._data, 'forEach')
recurse(this._data, [], me, fastCallback)
deepForEach(this._data, me, fastCallback)
}

/**
Expand Down
128 changes: 119 additions & 9 deletions src/utils/array.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { format } from './string.js'
import { DimensionError } from '../error/DimensionError.js'
import { IndexError } from '../error/IndexError.js'
import { deepStrictEqual } from './object.js'
import { findNumberOfArguments } from './optimizeCallback.js'

/**
* Calculate the size of a multi dimensional array.
Expand Down Expand Up @@ -834,15 +835,124 @@ export function get (array, index) {
* @param {Function} callback - Function that produces the element of the new Array, taking three arguments: the value of the element, the index of the element, and the Array being processed.
* @returns {*} The new array with each element being the result of the callback function.
*/
export function recurse (value, index, array, callback) {
if (Array.isArray(value)) {
return value.map(function (child, i) {
// we create a copy of the index array and append the new index value
return recurse(child, index.concat(i), array, callback)
})
} else {
// invoke the callback function with the right number of arguments
return callback(value, index, array)
export function deepMap (value, array, callback) {
const numberOfArguments = findNumberOfArguments(callback, array)
switch (numberOfArguments) {
case 1:
return recurse1(value)
case 2:
return recurse2(value, [])
case 3:
return recurse3(value, [])
default:
return recurse3(value, [])
}

function recurse1 (value) {
if (Array.isArray(value)) {
return value.map(function (child) {
// we create a copy of the index array and append the new index value
const results = recurse1(child)
return results
})
} else {
// invoke the callback function with the right number of arguments
return callback(value)
}
}

function recurse2 (value, index) {
if (Array.isArray(value)) {
return value.map(function (child, i) {
// we create a copy of the index array and append the new index value
index.push(i)
const results = recurse2(child, index)
index.pop()
return results
})
} else {
// invoke the callback function with the right number of arguments
return callback(value, [...index])
}
}

function recurse3 (value, index) {
if (Array.isArray(value)) {
return value.map(function (child, i) {
// we create a copy of the index array and append the new index value
index.push(i)
const results = recurse3(child, index)
index.pop()
return results
})
} else {
// invoke the callback function with the right number of arguments
return callback(value, [...index], array)
}
}
}

/**
* Recursive function to map a multi-dimensional array.
*
* @param {*} value - The current value being processed in the array.
* @param {Array} index - The index of the current value being processed in the array.
* @param {Array} array - The array being processed.
* @param {Function} callback - Function that produces the element of the new Array, taking three arguments: the value of the element, the index of the element, and the Array being processed.
* @returns {*} The new array with each element being the result of the callback function.
*/
export function deepForEach (value, array, callback) {
const numberOfArguments = findNumberOfArguments(callback, array)
switch (numberOfArguments) {
case 1:
recurse1(value)
break
case 2:
recurse2(value, [])
break
case 3:
recurse3(value, [])
break
default:
recurse3(value, [])
break
}

function recurse1 (value) {
if (Array.isArray(value)) {
value.forEach(function (child) {
recurse1(child)
})
} else {
// invoke the callback function with the right number of arguments
callback(value)
}
}

function recurse2 (value, index) {
if (Array.isArray(value)) {
value.forEach(function (child, i) {
index.push(i)
recurse2(child, index)
index.pop()
})
} else {
// invoke the callback function with the right number of arguments
callback(value, [...index])
}
}

function recurse3 (value, index) {
if (Array.isArray(value)) {
value.forEach(function (child, i) {
index.push(i)
recurse3(child, index)
index.pop()
})
} else {
// invoke the callback function with the right number of arguments
callback(value, [...index], array)
}
}
}

Expand Down
42 changes: 28 additions & 14 deletions src/utils/collection.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,15 @@ export function containsCollections (array) {
*/
export function deepForEach (array, callback) {
if (isMatrix(array)) {
array = array.valueOf()
recurse(array.valueOf())
} else {
recurse(array)
}

for (let i = 0, ii = array.length; i < ii; i++) {
const value = array[i]

if (Array.isArray(value)) {
deepForEach(value, callback)
function recurse (array) {
if (Array.isArray(array)) {
array.forEach(value => recurse(value))
} else {
callback(value)
callback(array)
}
}
}
Expand All @@ -54,13 +53,28 @@ export function deepForEach (array, callback) {
* @return {Array | Matrix} res
*/
export function deepMap (array, callback, skipZeros) {
if (array && (typeof array.map === 'function')) {
// TODO: replace array.map with a for loop to improve performance
return array.map(function (x) {
return deepMap(x, callback, skipZeros)
})
if (skipZeros) {
const callbackSkip = (x) => x === 0 ? 0 : callback(x)
if (isMatrix(array)) {
return array.create(recurse(array.valueOf(), callbackSkip), array.datatype())
} else {
return recurse(array, callbackSkip)
}
} else {
return callback(array)
if (isMatrix(array)) {
return array.create(recurse(array.valueOf(), callback), array.datatype())
} else {
return recurse(array, callback)
}
}
function recurse (array) {
if (Array.isArray(array)) {
return array.map(function (x) {
return recurse(x)
})
} else {
return callback(array)
}
}
}

Expand Down
20 changes: 15 additions & 5 deletions src/utils/optimizeCallback.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,27 @@ export function optimizeCallback (callback, array, name) {
const firstIndex = (array.isMatrix ? array.size() : arraySize(array)).map(() => 0)
const firstValue = array.isMatrix ? array.get(firstIndex) : get(array, firstIndex)
const hasSingleSignature = Object.keys(callback.signatures).length === 1
const numberOfArguments = _findNumberOfArguments(callback, firstValue, firstIndex, array)
const numberOfArguments = _typedFindNumberOfArguments(callback, firstValue, firstIndex, array)
const fastCallback = hasSingleSignature ? Object.values(callback.signatures)[0] : callback
if (numberOfArguments >= 1 && numberOfArguments <= 3) {
return (...args) => _tryFunctionWithArgs(fastCallback, args.slice(0, numberOfArguments), name, callback.name)
return (...args) => tryFunctionWithArgs(fastCallback, args.slice(0, numberOfArguments), name, callback.name)
}
return (...args) => _tryFunctionWithArgs(fastCallback, args, name, callback.name)
return (...args) => tryFunctionWithArgs(fastCallback, args, name, callback.name)
}
return callback
}

function _findNumberOfArguments (callback, value, index, array) {
export function findNumberOfArguments (callback, array) {
if (typed.isTypedFunction(callback)) {
const firstIndex = (array.isMatrix ? array.size() : arraySize(array)).map(() => 0)
const firstValue = array.isMatrix ? array.get(firstIndex) : get(array, firstIndex)
return _typedFindNumberOfArguments(callback, firstValue, firstIndex, array)
} else {
return callback.length
}
}

function _typedFindNumberOfArguments (callback, value, index, array) {
const testArgs = [value, index, array]
for (let i = 3; i > 0; i--) {
const args = testArgs.slice(0, i)
Expand All @@ -43,7 +53,7 @@ function _findNumberOfArguments (callback, value, index, array) {
* @returns {*} Returns the return value of the invoked signature
* @throws {TypeError} Throws an error when no matching signature was found
*/
function _tryFunctionWithArgs (func, args, mappingFnName, callbackName) {
export function tryFunctionWithArgs (func, args, mappingFnName, callbackName) {
try {
return func(...args)
} catch (err) {
Expand Down