tc39 / proposal-upsert

ECMAScript Proposal, specs, and reference implementation for Map.prototype.upsert
https://tc39.es/proposal-upsert/
MIT License
202 stars 14 forks source link

allocations / GC? #44

Closed alexpineda closed 1 month ago

alexpineda commented 2 years ago

I'd like to throw my 2c at this. I use a similar pattern described here in my WebGL game:

let unit = units.get(unitData.id);
if (!unit) {
  unit = createUnit(...);
  units.set(unit.id, unit);
}

So I have a few issues with the proposal:

  1. There is not necessarily a 3rd look up as you can see as the original variable can be simply assigned to.
  2. We're trading this lookup time for garbage creation, in the example provided there are one object and two functions?
    counts.emplace(key, {
    insert(key, map) {
    return 0;
    },
    update(existing, key, map) {
    return existing + 1;
    }
    });

If a consumer like myself is executing this code in my case my logic runs at 30fps x 3400 units max = 102,000 times. So I don't think I would be able to use this feature without incurring unwanted GC.

ljharb commented 2 years ago

You don’t have to create the object inline; you’d presumably create it at a higher scope and reuse it your 102k times.

runspired commented 1 year ago

@ljharb that presumes the ability to update the context. The proposed API here works well when the generator/factory doesn't need key-specific params, but it does not work well when it does.

e.g. this is great:

const EMPLACE_FACTORY = {
  insert() {
    return new Thing();
  }
}

function getKey(map, key) {
  return map.emplace(key, EMPLACE_FACTORY);
}

However this pattern is problematic:

function getKey(map, key, initProps) {
  return map.emplace(key,{
    insert(key) {
      return new Thing(key, initProps);
    }
  });
}

In the past when I've built custom data-structures to solve these issues I've chosen an API that curries for this reason:

const myMap = new MapWithInsert( {
  insert(key, ...args) {
    return new Thing(...args);
  }
});

function getKey(map, key, initProps) {
  return map.emplace(key, initProps);
}
ljharb commented 1 year ago

@runspired why is that pattern problematic? Currying requires you to create a new function every time, so I'd think it's much worse than the "problematic" pattern you describe.

runspired commented 1 year ago

It’s the middle one that forces creation of a new object, a field assignment, and a new function every time.

the last one I described as currying I should have described differently. You define the function once and you pass additional invoke args to emplace: implemented properly this requires no additional assignments, allocs or scopes.

ljharb commented 1 year ago

The cost of those is negligible, otherwise builtin APIs wouldn't constantly be taking options objects.

runspired commented 1 year ago

@ljharb ... that is just plainly untrue. It flies in the face of real experience optimizing hot paths, and hot-paths tends to be where Maps like this exist.

Here is a very crude benchmark that shows that the cost of those allocations is indeed highly significant in comparison to the cost of the static options or forwarded arguments.

https://jsbench.me/72lbhofcoa/1

dminor commented 1 month ago

The new design doesn't have an options object.