ljharb / js-traverse

MIT License
44 stars 8 forks source link

Traverse object with Buffer field is SLOW #9

Open jekanik opened 1 month ago

jekanik commented 1 month ago

Hi @ljharb! We are using traverse in case where we need to remove sensitive or inappropriate data from object before logging it. And there are cases when we have Buffer field in this object. We assuming that this field is sensitive and trying to filter it out. But, unfortunately, CPU profile showed that service spends ~500ms of CPU time inside of traverse.

How to reproduce:

    const obj = {
        data: readFileSync(join(__dirname, 'image.webp')), // image is 1.8MB
    };

    console.time('test_timer');

    const updated = traverse(obj).map(function () {
        if (this.key !== undefined && this.key === 'data') {
            this.update('secret');
        }
    });

    console.timeEnd('test_timer');

Logs: v. 0.6.6

test_timer: 331 ms

v. 0.6.9

test_timer: 116 ms

Update of the lib makes it better but still it's weird to have 100+ms latency for such case.

Could you please advice, if we are misusing lib or something else? Thanks

jekanik commented 1 month ago

Small addition - CPU profile (meaningful part)

image
ljharb commented 1 month ago

I'm assuming that when you say "ByteBuffer" you're referring to a node Buffer object?

ljharb commented 1 month ago

Potentially #2 is related.

jekanik commented 1 month ago

I'm assuming that when you say "ByteBuffer" you're referring to a node Buffer object?

Indeed. My Java experience accidentally kicked in :)

Potentially https://github.com/ljharb/js-traverse/pull/2 is related.

Tbh I'd prefer not to copy 1.8MB buffer :) P.S.: As I see map copies object, but not forEach. And still with forEach timing is similar.

Thanks for response @ljharb

ljharb commented 4 weeks ago

To be clear, the issue isn't about copying, it's about traversing objects that are "meant" to be treated as leaves/atoms, like a Buffer or a URL (see #10).