Don't allocate response objects to avoid triggering GC

[tobie]

On the list, @Maksims writes:

Garbage Collection - one of the major bottlenecks and high consideration for real-time applications. If application is targeting 60+ fps, it has to avoid any allocations and avoid calling any native JS methods that allocate response objects, for example getBoundingClientRect. High GC loops do lead to periodical JS thread stalls, that can drop many frames, making 60fps unreachable.

[tobie]

The original API design as proposed by @rwaldron didn't have GC collection issues. The new design does, mainly because I wanted the same API to be available from the event object and the sensor instance itself.

Now that we've stopped exposing the sensor reading from the event object, maybe we should reconsider exposing it as a separate object on the sensor itself to avoid GC issues.

What if we tried something like that instead (here for gyroscope)?:

[SecureContext]
interface Sensor : EventTarget {
  readonly attribute SensorState state;
  readonly attribute DOMHighResTimeStamp timestamp;
  void start();
  void stop();
  attribute EventHandler onchange;
  attribute EventHandler onactivate;
  attribute EventHandler onerror;
};

dictionary SensorOptions {
  double? frequency;
};

[Constructor(SensorOptions options)]
interface Gyroscope : Sensor {
  serializer = { x, y, z, timestamp }; // so we can still get a reading object when needed
  readonly attribute unrestricted double? x;
  readonly attribute unrestricted double? y;
  readonly attribute unrestricted double? z;
};

That would allow uses like this:

let gyro = new Gyroscope({ frequency: 240 });
gyro.start();
    
gyro.onchange = _ => {
    console.log("Rotation rate around the X-axis " + gyro.x);
    console.log("Rotation rate around the Y-axis " + gyro.y);
    console.log("Rotation rate around the Z-axis " + gyro.z);
};

gyro.onerror = event => console.log(event.error.name, event.error.message)

Thoughts?

[Maksims]

@tobie, this design would allow getting state of data, without allocation on changes. And would allow to subscribe to data changes. Which ticks both use cases: async and sync. And prevents allocations.

This is actually is a good option here.

Although, this pattern will only work for singleton sensor types - there is only one gyro, and one accelerometer ever? Does Sensors API assumes that there might be multiple sensors of the same type? Lets say there are two controllers, like HTC Vive Controller, so you can have multiple attached. Does Sensors API covers this types of sensors too?

One slight concern, with sensors that have frequency that affects the "accumulated" data. For example accelerometer would not just get current state, but accumulated between pulls data? If that is the case, then with sync use cases, pulling could be sync too.

[tobie]

Although, this pattern will only work for singleton sensor types - there is only one gyro, and one accelerometer ever? Does Sensors API assumes that there might be multiple sensors of the same type? Lets say there are two controllers, like HTC Vive Controller, so you can have multiple attached. Does Sensors API covers this types of sensors too?

Yes, via identifying parameters which are sensor type specific, e.g. (completely made-up identifying parameters):

let gyro = new Gyroscope({
    frequency: 240,
    external: true, // identifying parameter
    hand: "right"   // identifying parameter
});

One slight concern, with sensors that have frequency that affects the "accumulated" data. For example accelerometer would not just get current state, but accumulated between pulls data? If that is the case, then with sync use cases, pulling could be sync too.

I've punted on dealing with accumulate date for now, tbh. I wan't to make sure we don't design something that makes this impossible in the future, however, so suggestions are welcome.

[Maksims]

Yes, via identifying parameters which are sensor type specific, e.g. (completely made-up identifying parameters):

That makes good sense. Providing default behaviour assuming it subscribes to "first", and allowing devs to enumerate available sensors and choose specific.

I've punted on dealing with accumulate date for now, tbh. I wan't to make sure we don't design something that makes this impossible in the future, however, so suggestions are welcome.

Indeed, it can be a tricky one. So for example with Accelerometer, current values of acceleration - is what will be available as state, but not the deltas. Developer has ability to calculate delta if he needs so in event handlers or in fixed update method (sync path). Which makes sync polling not a need.

[tobie]

That makes good sense. Providing default behaviour assuming it subscribes to "first", and allowing devs to enumerate available sensors and choose specific.

Specific sensor specs get to define what default should be (or if a default makes sense at all).

[tobie]

Just to clarify, gyro.x is a getter for the last, asynchronously polled x value of the gyroscope, which is stored in a shared memory buffer. It is not a sync poller.

[Maksims]

Just to clarify, gyro.x is a getter for the last, asynchronously polled x value of the gyroscope, which is stored in a shared memory buffer. It is not a sync poller.

Indeed, this is just accessor to latest available data. If it would pull actual data, it then shall be a method to clearly indicate that there is logic going on behind the scenes, as it would block JS thread probably.

[alexshalamov]

@tobie Initially we've implemented API in most efficient way. The Sensor.reading was a 'current' reading, thus, no need to create new reading on every update. After discussion under (https://github.com/w3c/ambient-light/issues/15), you explained how you want API to behave and that SensorReading has to be recreated for every update.

So, I would propose to keep API as it is. If we want more FPS and less GC, we have to modify Sensor.reading description that will explain that Sensor.reading always points to current reading of the sensor.

'Sensor has reading' => interface Sensor { SensorReading reading; } (good design) 'Sensor is reading' => interface Sensor : SensorReading {} (bad design)

which is stored in a shared memory buffer. It is not a sync poller.

I know that Mikhail and I explained how we've implemented sensor reading update in Chromium, however, imo, we should avoid talking about UA specific implementation details.

[pozdnyakov]

Why not just keep the existing semantics (sensor.reading property) and say that it always returns the same object instead of re-creating it every round? Mixing of sensor and it's data does not look beneficial.

[tobie]

Thanks for your input @alexshalamov. Getting back to our requirements, I think we want to try and fulfill the following, somewhat contradictory use-cases:

1. high-frequency low-latency use-cases (e.g. head-tracking for VR),
2. operating over multiple, sequential sensor readings in (near-) realtime (e.g. Kalman filters),
3. storing sensor reading for further processing at a later stage, either locally or remotely.

Requirement 1) is extremely perf sensitive, and if our ability to deliver on it is hindered because we're triggering too much GC, we should definitely fix that. Requirements 2) and 3) on the other hand require that we have distinct objects representing each readings (additionally, timestamps for 3) need to have an epoch-based component and not be DOMHighResolution only).

The design I'm suggesting here (which, btw, is just a suggestion), delivers on 1), but require an extra bit of work for 2) and 3); basically a way to snapshot the state of the sensor at t_n. I added a serializer in the above example as a placeholder. But that could also be a method, e.g.:

[SecureContext]
interface Sensor : EventTarget {
  readonly attribute SensorState state;
  void start();
  void stop();
  SensorReading snapshot();
  attribute EventHandler onchange;
  attribute EventHandler onactivate;
  attribute EventHandler onerror;
};

interface SensorReading {
  readonly attribute DOMHighResTimeStamp timeStamp;
  readonly attribute DOMHighResTimeStamp timeOrigin;
};

dictionary SensorOptions {
  double? frequency;
};

// E.g. for Gyroscope

interface GyroscopeValues {
  readonly attribute unrestricted double? x;
  readonly attribute unrestricted double? y;
  readonly attribute unrestricted double? z;
};

[Constructor(SensorOptions options)]
interface Gyroscope : Sensor {};
Gyroscope implements GyroscopeValues;

interface GyroscopeReading : SensorReading {};
GyroscopeReading implements GyroscopeValues;

We could even imagine building on top of this a buffer for all snapshots in between to event turns or some such.

[tobie]

Why not just keep the existing semantics (sensor.reading property) and say that it always returns the same object instead of re-creating it every round?

Because it doesn't help us with requirements 2) and 3) above, makes for more verbose code (sensor.reading.x instead of sensor.x), and makes mutable an object developers will/should think of as immutable.

[tobie]

@rwaldron would love your thoughts on the above. Thanks!

[pozdnyakov]

Because it doesn't help us with requirements 2) and 3) above, makes for more verbose code (sensor.reading.x instead of sensor.x)

As for 2) and 3) sensor.reading.x is not any worse than sensor.x (is it?), however sensor.x (y, z) would semantically mean position of the sensor itself, not the obtained data.

and makes mutable an object developers will/should think of as immutable.

cannot reading be readonly?

[tobie]

Because it doesn't help us with requirements 2) and 3) above, makes for more verbose code (sensor.reading.x instead of sensor.x)

As for 2) and 3) sensor.reading.x is not any worse than sensor.x (is it?)

Well for both cases 2) and 3) it seems developers would essentially play around with reading objects, so they woudl be doing: reading.x anyway.

however sensor.x (y, z) would semantically mean position of the sensor itself, not the obtained data.

Meh. I'm sure that won't be an issue for JS devs at all. There's a long tradition of favoring terseness over semantic purity in JS. They'll feel right at home with this.

and makes mutable an object developers will/should think of as immutable.

cannot reading be readonly?

Sure, but that still wouldn't resolve immutability issues. It's no longer a sensor reading if it changes over time. It's a proxy for the sensor values.

[Maksims]

If sensor has reading property, which is an object, with properties, then developer in event might assume it is immutable and/or instanced. This could lead to confusion like:

var lastReading = null;
accelerometer.onchange = function() {
    if (lastReading) {
        var deltaX = this.reading.x - lastReading.x;
    }
    lastReading = this.reading;
};

Someone would be confused why this wouldn't work on assumption that reading was immutable objects.

But if there is no object involved, such as sensor.x, then:

var lastX = null;
accelerometer.onchange = function() {
    if (lastX !== null) {
        var deltaX = this.x - lastX;
    }
    lastX = this.x;
};

There is much less chance to get confused in this case. Bear in mind, examples above - are for async workflow. Sync workflow would access accelerometer data directly in fixed update function:

var lastX = null;
var update = function() {
    requestAnimationFrame(update);

    if (lastX !== null) {
        var deltaX = accelerometer.x - lastX;
    }
    lastX = accelerometer.x;
};
requestAnimationFrame(update);

[rwaldron]

But if there is no object involved, such as sensor.x, then: ... There is much less chance to get confused in this case.

Yep, that's how J5 is designed as well.

---

I've never been a fan of the sensor.reading.foo design, but was always willing to accept that certain changes would need to be made to accomodate the platform. I'm happy to see that we're circling back before fully committing to the existing design.

The design that @tobie suggested above, which had this example:

let gyro = new Gyroscope({ frequency: 240 });
gyro.start();
    
gyro.onchange = _ => {
    console.log("Rotation rate around the X-axis " + gyro.x);
    console.log("Rotation rate around the Y-axis " + gyro.y);
    console.log("Rotation rate around the Z-axis " + gyro.z);
};

gyro.onerror = event => console.log(event.error.name, event.error.message);

Satisfies criteria 1, 2 and 3. Here's how:

1. high-frequency low-latency use-cases (e.g. head-tracking for VR),

We already know why, it's because there is no excessive allocation and therefore no excessive GC demands.

2. operating over multiple, sequential sensor readings in (near-) realtime (e.g. Kalman filters),
3. storing sensor reading for further processing at a later stage, either locally or remotely.

Application code can easily copy the values from the properties and store them for later, if they want to do so:

let cache = [];
gyro.onchange = _ => {
    cache.push({ x, y, z } = gyro);
};

... Any amount of historic data can be kept or discarded as needed by the application (re: criteria 2)

[tobie]

Application code can easily copy the values from the properties and store them for later, if they want to do so

@rwaldron, seems you're suggesting ES6/7 provides enough syntactic sugar to make a gyro.snapshot() method (placeholder name) redundant. We can always add it at a later stage if lots of people ask for it.

[rwaldron]

seems you're suggesting ES6/7 provides enough syntactic sugar to make a gyro.snapshot()

Definitely

[alexshalamov]

@Maksims do you have any test set (data) which shows that current approach makes 60fps target unreachable? I quckly measured codepath for sensor reading update using current API and with modified codepath that matches proposal in this issue. The difference between two is neglectable: fluctuations on a level of 0.1-1 microseconds per reading update. Tested on Mi Pad2 device.

@tobie

Requirement 1) is extremely perf sensitive, and if our ability to deliver on it is hindered because we're triggering too much GC, we should definitely fix that.

Before changing the API, we need some data which proves that performance issues do exist, otherwise, it looks like a premature optimisation.

[tobie]

Garbage collection pauses causing missed frames are well known and well documented issues. Note the issue isn't the extra allocation time, it's the GC pauses.

Among the TAG requests (coming from @slightlyoff) was a requirement to make this available off of the main thread for fear of jank.

Designing CPU-hungry APIs designed for perf-sensitive applications with performance in mind isn't premature optimization, it's just Doing the Right Thing[TM]. ;)

That said, I'd love to see test data if there's some easily available, but I don't think the onus is on the OP to prove his point here, rather it's on us to explain how the current design is going to avoid causing GC pauses when GC kicks in.

[Maksims]

@alexshalamov @tobie - have answered pretty well. GC stalls is very known problem working with real-time apps. Current design on it's own wont lead to long GC, but it makes GC longer, which means there is less GC "buffer" for application logic. If application will go over that "buffer" of what GC can keep up with, it will get into Major GC long stalls and freeze every second or so. With only one solution - refresh the tab. Remember that GC is shared between tabs of same JS thread (when opening pages in new tabs within same domain). This means multiple apps will contribute even more to GC.

Every little thing matters. And if API does not provides a way to avoid allocations then developers hitting the GC issues will struggle to deal with a situation.

As @tobie said:

Designing CPU-hungry APIs designed for perf-sensitive applications with performance in mind isn't premature optimization, it's just Doing the Right Thing[TM]. ;)

[alexshalamov]

@tobie There are few concerns with proposed modifications.

• When control and data interfaces are merged, extensibility is lost. For example, for sensors that can provide additional data in 'uncalibrated / raw mode' either new interface should be created, or all possible data fields have to be present on main interface.

• snapshot() makes new object, I don’t see any difference to what we have at the moment.

• timeStamp is only accessible through snapshot() => for algorithms that use timestamp, performance would be the same as with the current API.

Chrome canary has implementation of ambient light, accelerometer, gyroscope and magnetometer. It would be good to have measurements that prove that the current approach would not allow us to reach continuous 60fps target. If required, I can quickly prototype infinite scroller with recyclable elements, so that we will see whether current approach affecting scrolling performance / fps or not.

If you still think that GC would be a problem, we could consider an alternative solution. Could we instead provide a method that will fill sensor reading data, e.g. Sensor.getReading(Dictionary out); or Sensor.read(Dictionary out);? This will allow to reuse object, yet, main interface is not polluted with unwanted fields and extensibility is not lost.

[Maksims]

Could we instead provide a method that will fill sensor reading data, e.g. Sensor.getReading(Dictionary out); or Sensor.read(Dictionary out);? This will allow to reuse object, yet, main interface is not polluted with unwanted fields and extensibility is not lost.

This will require extra function call by a developer every time he wants to get some data out of a sensor? And it requires preparation - need to create object to store readings. Haven't seen such pattern anywhere in a W3C.

Additionally, it is not necessary all readings data is required by developer, he might want to store only some of it. So clearly there will be overhead when all of it is not needed.

Direct access by either sensor.x or sensor.data.x - provides ability to store data developer wants, and access to the data without a need to call a function and no need for any advance preparations. Plus this avoids any allocations.

[alexshalamov]

@Maksims

Haven't seen such pattern anywhere in a W3C.

https://w3c.github.io/webvr/#dom-vrdisplay-getframedata

Since you have raised concerns about GC / performance issues related to current API, do you have any data / test app that shows the problem? As I mentioned before, you can test Generic Sensor based APIs with Chromium canary builds by enabling "Generic Sensor" flag.

sensor.data.x

How is that different from sensor.reading.x?

[tobie]

Replying to @alexshalamov:

@tobie There are few concerns with proposed modifications.

• When control and data interfaces are merged, extensibility is lost. For example, for sensors that can provide additional data in 'uncalibrated / raw mode' either new interface should be created, or all possible data fields have to be present on main interface.

That doesn't bother me, frankly. If you have options that change the behavior of the sensor, it seems appropriate that those would be reflected on the data. My feeling is that for the case where this will really become an issue, the right answer is probably to create different interfaces altogether.

• snapshot() makes new object, I don’t see any difference to what we have at the moment.

Yeah. That was for the case where a snapshot is needed. E.g. for storing readings. @rwaldron indicates above that ES6 syntactic sugar makes that redundant, though. So let's drop it.

• timeStamp is only accessible through snapshot() => for algorithms that use timestamp, performance would be the same as with the current API.

You're right. This belongs on the main API too. Need to find a better name for it, though.

Chrome canary has implementation of ambient light, accelerometer, gyroscope and magnetometer. It would be good to have measurements that prove that the current approach would not allow us to reach continuous 60fps target. If required, I can quickly prototype infinite scroller with recyclable elements, so that we will see whether current approach affecting scrolling performance / fps or not.

I don't think there's any doubt that GC pauses is a serious issue for high perf real-time use-cases. This has been the case for a decade in the Web, is one of the motivations behind ASM.js and WASM (or however that thing is called nowadays), and is the reason game developers usually pool resources on GC platforms. That said, more perf tests is always a good thing.

In order to measure the impact of GC on perf, I suggest looking at impact on a real game on low-end devices. Maybe you could look at something like this gamebench: http://www.scirra.com/demos/c2/sbperftest/ and see how it fares on a low end Android device once you add 120 new gyroscope objects per second and whatever the max polling rate for magnetometer is.

If you still think that GC would be a problem, we could consider an alternative solution. Could we instead provide a method that will fill sensor reading data, e.g. Sensor.getReading(Dictionary out); or Sensor.read(Dictionary out);? This will allow to reuse object, yet, main interface is not polluted with unwanted fields and extensibility is not lost.

Yeah, I'm not hostile to the "bring your own buffer" design pattern. iirc WebCrypto uses it quite a bit. It's a lot less user-friendly though, so it needs to have serious benefits beyond theoretical purity (we can't dismiss priority of constituencies).

[tobie]

Replying to @Maksims:

Could we instead provide a method that will fill sensor reading data, e.g. Sensor.getReading(Dictionary out); or Sensor.read(Dictionary out);? This will allow to reuse object, yet, main interface is not polluted with unwanted fields and extensibility is not lost.

This will require extra function call by a developer every time he wants to get some data out of a sensor? And it requires preparation - need to create object to store readings.

I don't think a function call is a huge perf drag nowadays. It was in the days of IE6, but those are behind us at this point.

Direct access by either sensor.x or sensor.data.x - provides ability to store data developer wants, and access to the data without a need to call a function and no need for any advance preparations.

These are getters, so in terms of cost, we're not far away from function calls afaik.

Plus this avoids any allocations.

Yes. That's the issue we want to avoid. The function call one is a red herring.

[tobie]

Replying to @alexshalamov:

Since you have raised concerns about GC / performance issues related to current API, do you have any data / test app that shows the problem?

As mentioned earlier, GC-pauses in real-time applications is a know issue. The onus isn't on the OP to prove it is. It's on whoever pushes an API that allocates new object on every reading to prove it isn't. I don't volunteer. :-P

[tobie]

Taking the above comments in consideration, here's a new take at the WebIDL for this:

[SecureContext]
interface Sensor : EventTarget {
  readonly attribute SensorState state;
  readonly attribute DOMHighResTimeStamp lastUpdatedAt; // better name?
  readonly attribute DOMHighResTimeStamp timeOrigin;
  void start();
  void stop();
  attribute EventHandler onchange; // onupdate? onread? onreading? onpoll?
  attribute EventHandler onactivate;
  attribute EventHandler onerror;
};

dictionary SensorOptions {
  double? frequency;
};

// E.g. for Gyroscope

[Constructor(SensorOptions options)]
interface Gyroscope : Sensor {
  readonly attribute unrestricted double? x;
  readonly attribute unrestricted double? y;
  readonly attribute unrestricted double? z;
};

[Maksims]

I don't think a function call is a huge perf drag nowadays. It was in the days of IE6, but those are behind us at this point.

I'm more relating to ease of use and friendliness of an API, where learners don't need to research much of how to use it, learning new concepts. Basically, simply getting data from properties is easier than allocating extra object and getting readings to that object every time data is required. This makes usability somewhat less simple.

[rwaldron]

Basically, simply getting data from properties is easier than allocating extra object and getting readings to that object every time data is required. This makes usability somewhat less simple.

This is also supported by four years of J5 use in the wild—developers "get it".