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 14 commits into
base: develop
Choose a base branch
from
5 changes: 3 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 @@ -52,5 +52,6 @@ export const createForEach = /* #__PURE__ */ factory(name, dependencies, ({ type
* @private
*/
function _forEach (array, callback) {
recurse(array, [], array, optimizeCallback(callback, array, name))
const fastCallback = optimizeCallback(callback, array, name, { detailedError: true })
deepForEach(array, array, fastCallback, fastCallback.length)
}
5 changes: 3 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,7 @@ export const createMap = /* #__PURE__ */ factory(name, dependencies, ({ typed })
* @private
*/
function _mapArray (array, callback) {
return recurse(array, [], array, optimizeCallback(callback, array, name))
const fastCallback = optimizeCallback(callback, array, name, { detailedError: true })
return deepMap(array, array, fastCallback, fastCallback.length)
}
})
142 changes: 133 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,138 @@ 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, numberOfArguments) {
const size = arraySize(value)
const N = size.length - 1
switch (numberOfArguments || findNumberOfArguments(callback, array)) {
case 1:
return recurse1(value, 0)
case 2:
return recurse2(value, size.map(() => null), 0)
case 3:
return recurse3(value, size.map(() => null), 0)
default:
return recurse3(value, size.map(() => null), 0)
}

function recurse1 (value, depth) {
if (depth < N) {
return value.map(function (child) {
// we create a copy of the index array and append the new index value
const results = recurse1(child, depth + 1)
return results
})
} else {
// invoke the callback function with the right number of arguments
return value.map(v => callback(v))
}
}

function recurse2 (value, index, depth) {
if (depth < N) {
return value.map(function (child, i) {
// we create a copy of the index array and append the new index value
index[depth] = i
const results = recurse2(child, index, depth + 1)
index[depth] = null
return results
})
} else {
// invoke the callback function with the right number of arguments
return value.map((v, i) => {
index[depth] = i
return callback(v, index.slice())
})
}
}

function recurse3 (value, index, depth) {
if (depth < N) {
return value.map(function (child, i) {
// we create a copy of the index array and append the new index value
index[depth] = i
const results = recurse3(child, index, depth + 1)
index[depth] = null
return results
})
} else {
// invoke the callback function with the right number of arguments
return value.map((v, i) => {
index[depth] = i
return callback(v, index.slice(), 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, numberOfArguments) {
const size = arraySize(value)
const N = size.length - 1
switch (numberOfArguments || findNumberOfArguments(callback, array)) {
case 1:
recurse1(value, 0)
break
case 2:
recurse2(value, size.map(() => null), 0)
break
case 3:
recurse3(value, size.map(() => null), 0)
break
default:
recurse3(value, size.map(() => null), 0)
break
}

function recurse1 (value, depth) {
if (depth < N) {
value.forEach(function (child) {
recurse1(child, depth + 1)
})
} else {
// invoke the callback function with the right number of arguments
value.forEach(v => callback(v))
}
}

function recurse2 (value, index, depth) {
if (depth < N) {
value.forEach(function (child, i) {
index[depth] = i
recurse2(child, index, depth + 1)
index[depth] = null
})
} else {
// invoke the callback function with the right number of arguments
value.forEach((v, i) => {
index[depth] = i
callback(v, index.slice())
})
}
}

function recurse3 (value, index, depth) {
if (depth < N) {
value.forEach(function (child, i) {
index[depth] = i
recurse3(child, index, depth + 1)
index[depth] = null
})
} else {
// invoke the callback function with the right number of arguments
value.forEach((v, i) => {
index[depth] = i
callback(v, index.slice(), 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
49 changes: 40 additions & 9 deletions src/utils/optimizeCallback.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,24 +8,55 @@ import { typeOf as _typeOf } from './is.js'
* @param {Function} callback The original callback function to simplify.
* @param {Array|Matrix} array The array that will be used with the callback function.
* @param {string} name The name of the function that is using the callback.
* @returns {Function} Returns a simplified version of the callback function.
* @returns {Array} Returns an array with the simplified version of the callback function and it's number of arguments
*/
export function optimizeCallback (callback, array, name) {
export function optimizeCallback (callback, array, name, options) {
if (typed.isTypedFunction(callback)) {
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 fastCallback = hasSingleSignature ? Object.values(callback.signatures)[0] : callback
if (numberOfArguments >= 1 && numberOfArguments <= 3) {
return (...args) => _tryFunctionWithArgs(fastCallback, args.slice(0, numberOfArguments), name, callback.name)
const numberOfArguments = _typedFindNumberOfArguments(callback, firstValue, firstIndex, array)
if (options && options.detailedError) {
const fastCallback = hasSingleSignature ? Object.values(callback.signatures)[0] : callback
switch (numberOfArguments) {
case 1:
return (val) => tryFunctionWithArgs(fastCallback, [val], name, callback.name)
case 2:
return (val, idx) => tryFunctionWithArgs(fastCallback, [val, idx], name, callback.name)
case 3:
return (val, idx, array) => tryFunctionWithArgs(fastCallback, [val, idx, array], name, callback.name)
default:
return (...args) => tryFunctionWithArgs(fastCallback, args, name, callback.name)
}
} else if (hasSingleSignature) {
return Object.values(callback.signatures)[0]
} else {
switch (numberOfArguments) {
case 1:
return val => callback(val)
case 2:
return (val, idx) => callback(val, idx)
case 3:
return (val, idx, array) => callback(val, idx, array)
default:
return callback
}
}
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 +74,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
31 changes: 28 additions & 3 deletions test/benchmark/forEach.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,6 @@ const genericMatrix = map(ones(10, 10, 'dense'), _ => round(random(-5, 5), 2))
const numberMatrix = new DenseMatrix(genericMatrix, 'number')
const array = genericMatrix.toArray()

// console.log('data', array)
// console.log('abs(data)', abs(array))npm run

const bench = new Bench({ time: 100, iterations: 100 })
.add('abs(genericMatrix)', () => {
abs(genericMatrix)
Expand Down Expand Up @@ -43,6 +40,34 @@ const bench = new Bench({ time: 100, iterations: 100 })
.add('numberMatrix.forEach(abs.signatures.number)', () => {
numberMatrix.forEach(abs.signatures.number)
})
.add('genericMatrix.forEach(abs+idx)', () => {
genericMatrix.forEach((x, idx) => abs(x) + idx[0] - idx[1])
})
.add('numberMatrix.forEach(abs+idx)', () => {
numberMatrix.forEach((x, idx) => abs(x) + idx[0] - idx[1])
})
.add('forEach(genericMatrix, abs+idx)', () => {
forEach(genericMatrix, (x, idx) => abs(x) + idx[0] - idx[1])
})
.add('genericMatrix.forEach(abs+idx+arr)', () => {
genericMatrix.forEach((x, idx, X) => abs(x) + idx[0] - idx[1] + X.get([0, 0]))
})
.add('numberMatrix.forEach(abs+idx+arr)', () => {
numberMatrix.forEach((x, idx, X) => abs(x) + idx[0] - idx[1] + X.get([0, 0]))
})
.add('forEach(genericMatrix, abs+idx+arr)', () => {
forEach(genericMatrix, (x, idx, X) => abs(x) + idx[0] - idx[1] + X.get([0, 0]))
})
.add('forEach(array, abs+idx+arr)', () => {
forEach(array, (x, idx, X) => abs(x) + idx[0] - idx[1] + X[0][0])
})
.add()
.on('cycle', function (event) {
console.log(String(event.target))
})
.on('complete', function () {
})
.run()

bench.addEventListener('cycle', (event) => console.log(formatTaskResult(bench, event.task)))
await bench.run()
Loading
Loading