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

Add fieldKey prop to <ArrayField> to improve performance on large arrays #4437

Merged
merged 5 commits into from
Mar 5, 2020
Merged

Add fieldKey prop to <ArrayField> to improve performance on large arrays #4437

merged 5 commits into from
Mar 5, 2020

Conversation

Hemant-yadav
Copy link
Contributor

Is your feature request related to a problem? Please describe.
using JSON.stringify results with a very large key. Passing a fieldKey when available reduces the overhead

@fzaninotto
Copy link
Member

Hi, and thanks for your patch.

Did you measure the actual overhead of the JSON stringification? Can you provide metrics showing the improvement due to your change?

@Hemant-yadav
Copy link
Contributor Author

Hi @fzaninotto

I have only taken the getDataAndIds function and passed the data with and without fieldKey.
Even with a smaller amount of data the difference is significant and with large array elements the difference would be greater.

const get = require('lodash/get');
const { performance } = require('perf_hooks');

const json = require('./data.json'); // Sample data { list: [Array]}  https://www.json-generator.com/ 
const N = 10;

function getDataAndIds(record, source, fieldKey) {
  const list = get(record, source);
  if (!list) {
    return initialState;
  }
  return fieldKey
    ? {
      data: list.reduce((prev, item) => {
        prev[item[fieldKey]] = item;
        return prev;
      }, {}),
      ids: list.map(item => item[fieldKey]),
    }
    : {
      data: list.reduce((prev, item) => {
        prev[JSON.stringify(item)] = item;
        return prev;
      }, {}),
      ids: list.map(JSON.stringify),
    };
}

// Without fieldKey
var p0 = process.memoryUsage().heapUsed;
var t0 = performance.now();
for (i = 0; i < N; i++)
  getDataAndIds(json, 'list')
var t1 = performance.now();
var p1 = process.memoryUsage().heapUsed;

// With fieldKey
var p2 = process.memoryUsage().heapUsed;
var t2 = performance.now();
for (j = 0; j < N; j++)
  getDataAndIds(json, 'list', '_id')
var t3 = performance.now();
var p3 = process.memoryUsage().heapUsed;
> sw_vers
ProductName:    Mac OS X
ProductVersion: 10.15.2
BuildVersion:   19C57
> node -v
 v10.18.0
N Avg Time Taken in ms Without Field Key (t1 - t0) Memory taken in Bytes without fieldKey (p1 - p0) Avg Time Taken in ms With Field Key (t3 - t2) Memory taken in Bytes with fieldKey (p3 - p2) Faster Memory Efficient
1 1.0340410023927689 69784 0.11160099878907204 3752 9x 19x
10 2.0885499976575375 369160 0.10323800146579742 8296 20x 44x
20 3.4119240008294582 702544 0.12716099992394447 13256 27x 53x
30 4.443512998521328 1034944 0.14266299828886986 18216 31x 57x

Thanks for your early reply and for this awesome project.

@fzaninotto
Copy link
Member

Thanks, but I meant real-life usage.

I have no doubt that skipping stringification is faster than using it. But my point is: on an average scenario of ArrayField usage, what's the overhead of the JSON stringification as compared to all the other JS execution?

My intuition is that, unless you're using ArrayField with a very large array of very large objects, the cost of stringifying JSON isn't significant when compared to the cost of rendering. I may be wrong of course, but before we fix a potential bug, can you show me the actual bug?

@Hemant-yadav
Copy link
Contributor Author

Hemant-yadav commented Feb 26, 2020

Hi,

With small data the difference is insignificant compared to rendering but with 50 records total of ~643KB data here are the results.

I have created a sandbox with the exact code I used.

Without FieldKey
withoutFieldKey

With FieldKey
withFieldKey

The time difference between rendering the ArrayField was [~10ms, ~30ms]and total rendering was also reduced by [~50ms, ~100ms]. The memory size is also reduced by ~1MB.

@fzaninotto
Copy link
Member

Thanks for taking the time. That's convincing, so we'll consider merging it.

But before that, you need to change the base branch to next (as it's not a bug fix), and add documentation for the new prop.

@Hemant-yadav Hemant-yadav changed the base branch from master to next February 27, 2020 09:48
Hemant-yadav and others added 3 commits February 27, 2020 15:44
using JSON.stringify results with a very large key. Passing a fieldKey when available reduces the overhead

Update ArrayField.tsx: Fix prettier errors

Update Array.tsx: put space for prettier
@fzaninotto fzaninotto changed the title Perf (ArrayField.tsx): Pass fieldKey to use as key Add fieldKey prop to <ArrayField> to improve performance for large arrays Mar 5, 2020
@fzaninotto fzaninotto changed the title Add fieldKey prop to <ArrayField> to improve performance for large arrays Add fieldKey prop to <ArrayField> to improve performance on large arrays Mar 5, 2020
@fzaninotto fzaninotto merged commit a503ade into marmelab:next Mar 5, 2020
@fzaninotto
Copy link
Member

Thanks!

@fzaninotto fzaninotto added this to the 3.3.0 milestone Mar 5, 2020
@Hemant-yadav
Copy link
Contributor Author

Thanks for docs help

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants