Open gefjon opened 2 weeks ago
Closes #666 . Nice.
benchmarks please
Benchmarking failed. Please check the workflow run for details.
Uhhh keep in mind callgrind might not simulate delays due to atomics. But if they're uncontended it probably doesn't matter.
Uhhh keep in mind callgrind might not simulate delays due to atomics. But if they're uncontended it probably doesn't matter.
Ah, I hadn't considered that. Are the wall-time benchmarks working again? Do I need to merge something into this branch?
Description of Changes
TxId
andMutTxId
now read from theInstant
clock around all of their public methods, and accumulate the totalDuration
spent in those methods throughout the life of the transaction. Then, when committing or aborting the transaction, theRelationalDB
charges energy via a newEnergyMonitor
method,record_datastore_time
.This necessitates having
RelationalDB
hold anArc<dyn EnergyMonitor>
, as previously it had no mechanism by which to access the energy monitor.Note that this approach is intended as a stopgap, rather than a permanent solution. Notable flaws include:
Energy consumed by datastore operations is not included in the WS broadcasts sent to clients; these contain only WASM execution energy. This is because we construct the
ModuleEvent
before downgrading the TX and running incremental queries, and the total datastore energy cost is only available after those operations.Wall time is a very noisy measurement. It is not necessarily incorrect to pass this noise on to our customers, since many of the unpredictable costs are caused by events like page allocations which are caused by the user code, and in general we expect any variance not caused by user code to disappear as the total time grows large. Still, it's not wonderful that we will end up charging different amounts of energy to run the same operations against the same data sets multiple times.
This patch does not track time spent in iterator or
RowRef
operations, only methods on the (Mut
-)TxId
(which are generally accessed through theRelationalDB
).We also need to take care that this does not significantly regress performance, since it involves a potentially large number of added clock reads and relaxed atomic increments. My hope is that the atomics are uncontended and so it's not a significant cost, but hey, this is why we have benchmarks, right?
API and ABI breaking changes
Will require private PR, which I will do after we determine that this is an acceptable approach and that the overhead is low. No user-facing interfaces change.
Expected complexity level and risk
2 - performance risks, and may under- or over-charge energy, but seems unlikely to break functionality.
Testing
Describe any testing you've done, and any testing you'd like your reviewers to do, so that you're confident that all the changes work as expected!