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 Optimierung Map vs Array #273

Closed
Arcticon opened this issue May 9, 2024 · 6 comments · Fixed by #279
Closed

Performance Optimierung Map vs Array #273

Arcticon opened this issue May 9, 2024 · 6 comments · Fixed by #279
Labels
enhancement New feature or request

Comments

@Arcticon
Copy link
Contributor

Arcticon commented May 9, 2024

Die function getDatapoint in main.js kann optimiert werden, weil immer nach einem unique key gesucht wird.
Wenn eine Map anstatt einem Array benutzt wird kann man konstante zugriffszeiten bekommen.

Ich habe das mal implementiert und einen benchmark laufen lassen:

cpu: AMD Ryzen 7 7800X3D 8-Core Processor
runtime: node v20.13.0 (x64-linux)

benchmark              time (avg)             (min … max)
---------------------------------------------------------
• best
---------------------------------------------------------
getDatapoint         3.14 ns/iter    (2.99 ns … 78.87 ns)
getDatapointForOf    2.31 ns/iter    (2.25 ns … 91.47 ns)
map getDatapoint      127 ps/iter       (117 ps … 114 ns) !

summary for best
  map getDatapoint
   18.25x faster than getDatapointForOf
   24.76x faster than getDatapoint

• worst
---------------------------------------------------------
getDatapoint        1'643 ns/iter   (1'585 ns … 2'275 ns)
getDatapointForOf       6 ns/iter    (5.36 ns … 35.53 ns)
map getDatapoint      127 ps/iter     (117 ps … 49.89 ns) !

summary for worst
  map getDatapoint
   47.24x faster than getDatapointForOf
   12933.04x faster than getDatapoint

best sucht nach channel (erstes element im array)
worst sucht nach ste1 (letztes element im array)

Code (bench.mjs):

import { baseline, bench, group, run } from 'mitata';
import { DATAPOINTS, mapGetDatapoint } from '../dysonConstants.js';

function getDatapoint(searchValue) {
  for (let row = 0; row < DATAPOINTS.length; row++) {
    if (
      DATAPOINTS[row].find(element => element === searchValue)
    ) {
      // this.log.debug('FOUND: ' + DATAPOINTS[row]);
      return DATAPOINTS[row];
    }
  }
}

function getDatapointForOf(searchValue) {
  for (const point of DATAPOINTS) {
    return point.find(element => element === searchValue)
  }
}

group('best', () => {
  const searchValue = 'channel';
  bench('getDatapoint', () => getDatapoint(searchValue));
  bench('getDatapointForOf', () => getDatapointForOf(searchValue));
  baseline('map getDatapoint', () => mapGetDatapoint(searchValue));
});

group('worst', () => {
  const searchValue = 'ste1';
  bench('getDatapoint', () => getDatapoint(searchValue));
  bench('getDatapointForOf', () => getDatapointForOf(searchValue));
  baseline('map getDatapoint', () => mapGetDatapoint(searchValue));
});

await run({
//   units: false, // print small units cheatsheet
  silent: false, // enable/disable stdout output
  avg: true, // enable/disable avg column (default: true)
  json: false, // enable/disable json output (default: false)
  colors: true, // enable/disable colors (default: true)
  min_max: true, // enable/disable min/max column (default: true)
  percentiles: true // enable/disable percentiles column (default: true)
});
Arcticon added a commit to Arcticon/ioBroker.dysonairpurifier that referenced this issue May 9, 2024
@Grizzelbee Grizzelbee reopened this May 13, 2024
@Grizzelbee
Copy link
Owner

Grizzelbee commented May 13, 2024

@Arcticon

Hmmm. Diese Änderung hat leider ein fettes Problem in den Adapter eingeschleust: [#278]

Das Problem liegt darin, dass das alte Array aus zwei Richtungen abgefragt wurde:

  1. Aus processMsg() über den vierstelligen dyson-code um die vom Gerät gesendeten Nachrichten zu entschlüsseln - das funktioniert auch aktuell noch.
  2. Aus onStateChange() um manuelle Änderungen im Objektbaum an das Gerät zu senden - das ist kaputt gegangen.

Analyse:
gekürzter Beispielcode

const datapoints = new Map([
	[
	  'channel',
	  {
		name: 'WIFIchannel',
		description: 'Number of the used WIFI channel.',
		type: 'number',
		writeable: false,
		role: 'value',
		unit: ''
	  }
	],
	[
	  'ercd',
	  {
		name: 'LastErrorCode',
		description: 'Error code of the last error occurred on this device',
		type: 'string',
		writeable: false,
		role: 'text',
		unit: ''
	  }
	]
]);


console.log( datapoints.get("LastErrorCode") );
console.log( datapoints.get("ercd") );

Ergebnis:

Node.js v20.12.2
PS C:\Users\xxx\WebstormProjects\ioBroker.dysonairpurifier> node ..\test.js
undefined // <- "LastErrorCode" kann nicht aufgelöst werden


// Ergebnis der Suche nach ERCD
{
  name: 'LastErrorCode',
  description: 'Error code of the last error occurred on this device',
  type: 'string',
  writeable: false,
  role: 'text',
  unit: ''
}

Wie bekommen wir es jetzt wieder hin auch nach "LastErrorCode" etc suchen zu können, ohne die Map (oder Teile davon) doppeln zu müssen?

Arcticon pushed a commit to Arcticon/ioBroker.dysonairpurifier that referenced this issue May 13, 2024
@Arcticon
Copy link
Contributor Author

@Grizzelbee ich habe noch eine lookup map hinzugefuegt. damit funktioniert wieder alles

@Grizzelbee
Copy link
Owner

Grizzelbee commented May 13, 2024

Habe ich gerade schon gesehen - mag die Lösung aber nicht, weil sie jeden einzelnen dyson-code und den zugehörigenb Obejektbaum-Namen doppelt - was bei künftigen Änderungen mit Sicherheit vergessen wird und zu Problemen führt.
Redundanz ist immer Mist.

Kann man diese Liste nicht dynamisch aus den Keys der Map und dem zugehörigen *.Name" bilden? Da würde ich zugunsten der Objekt-Update-Performanz mitspielen.

@Arcticon
Copy link
Contributor Author

Man koennte das hier machen:

new Map(
  Array.from(datapoints.entries()).map(([key, { name }]) => [name, key])
);

wie findest du das?

@Grizzelbee
Copy link
Owner

Yapp - finde ich gut.

Arcticon pushed a commit to Arcticon/ioBroker.dysonairpurifier that referenced this issue May 13, 2024
@Arcticon
Copy link
Contributor Author

@Grizzelbee habe noch tests hinzugefuegt.

@Grizzelbee Grizzelbee added the enhancement New feature or request label May 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants