uber-web / probe.gl Goto Github PK

Background: I'm running a benchmark test where I created an async function with a 5 second delay. The benchmark took 178s on average to run. If I lower the delay to 1 second, the benchmark took 30 seconds, when I increase the delay to 10 seconds, the benchmark took 318s.

Question: I'm curious why a 5 second function took 3 minutes to run, or a 10 second function took 5+ minutes to run. Is this a bug or is Bench intended to take this long?

Note: I noticed after further research that runBenchTestForMinimumTimeAsync has a multiplier of 10 by default on line 306 in bench.ts and it cannot be changed. Is there anything that can be done to be able to control that?

for #92 and #93

As it stands, the app instantiating a StatsWidget needs to know the names and types of individual stats in the stats object. This contradicts the idea of a Stats object containing a "bag" of useful but non-documented stats, and adds a bunch of fragile "boilerplate" code to apps.

Stats

• getTable method put things in a format consumable by console.table

StatsWidget

• Better handling of CSS
• Default formatter types, e.g. "time", "count", "framerate", "memory"
• Throttle counter: optional argument to only update every n calls to update
• Reset counter for individual stats: auto-reset every n calls to update
• Accept multiple stats objects

As I was looking through the bench.group function, I noticed that it doesn't ever set a test.group parameter to true. In the documentation, it seems to be implied that bench.group will add all tests under it to that group, although that is not what happens. I guess it's unclear when it says group(id) Adds a group header., because what does that actually mean? Some clarity around this would be helpful.

Background: The reason I bring this up is because I noticed benchmark tests are always run at least twice (once for calibration tests, and once for running the benchmarks). Under calibration, I noticed that they're run if test.group is set to false. So should I always expect calibration tests to be run? The only way I have found to only run a benchmark test once is when I set test.group to true.

The documentation I am referring to: https://uber-web.github.io/probe.gl/docs/api-reference/bench/bench

Repros in https://loaders.gl/examples/3d-tiles

See here:
https://github.com/uber-web/probe.gl/blob/master/modules/log/src/utils/hi-res-timestamp.ts#L8

The imported isBrowser is a function that returns a boolean. It looks like that should be called isBrowser()

Turns out there are two, nearly-identical, high-resolution timers in probe:
https://github.com/uber-web/probe.gl/blob/master/modules/bench/src/timer.js
https://github.com/uber-web/probe.gl/blob/master/modules/core/src/utils/timestamp.js

These can be consolidated into one that can be re-used.

• Many docs are still marked as WIP
• get-started section should be built into Developer's Guide
• doc/get-started structure doesn't well reflect the multiple modules

This happens for instance in 3d-tiles demo, where each Tileset3D has a stats object, and the StatsWidget need to be updated when a new tileset is loaded.

Either provide

• a way to reset the stats object, or
• a way to destroy the stats widget (so that a new one can be created)
  or better yet, provide both...

This code breaks during server side HTML generation, the document reference needs to be "protected"

Instead of increment count etc, 3d tiles often just wants to update a stat to a specific value.

Probe.gl currently offers a spy utility but lacks support for stubbing. Stubbing is really helpful for overriding the behavior of methods within tests. For more info I'd point to the sinon documentation.

For discussion:

In probe we already store log settings in local storage.

We could also have StatsWidgets hidden by default and offer a console command to enable them.

> probe.stats = true // or
> probe.showStats()

This way, devs could easily turn on stats after a new release or when they think they see regressions.

https://www.npmjs.com/package/@probe.gl/stats-widget/v/3.0.0-alpha.9

At least we should have a minimal two-line README that links to our website.

Deck usage requires gathering measurements that only become visible after a certain number of measurements. The Stats object should support this usage. I think the simplest way to do this would that the Stats object should have a windowSize property. Calls to stat updates like incrementCount, addTime, etc. should only update private members _count and _time. count and time properties would only be updated when _count % windowSize === 0.

@Pessimistress this should support the usage you need for Deck without _onMetrics.

I'm noticing that my first test that is benchmarked is slower than the rest because of the calibration tests (my assumption). Is there a way to disable this?

As of uber-web/ocular#197, published modules properly include their full license text. deck.gl has published v7.0.10 in response, can this repo do the same? Thanks!

The headings (“Get Started”, “Articles”, "API Reference”) in your documentation site cannot be navigated in Safari (11.1.2 (13605.3.8), or Technology Preview Release 63 (Safari 12.1, WebKit 13607.1.2.1)). Instead, you’re navigated back to the overview.

I can't access either the uber-web gh pages or http://probe.gl/#/documentation/overview (and am failing to build the docs locally with docusaurus!)

using inline functions for conditional logging take 2-3x more time than unconditionally building strings.

Looks like the function parameter style does not provide the intended performance benefits, see visgl/deck.gl#3949

Proposals:

• modify the various log calls to take multiple string segment as parameters and concat them inside the log functions when logging actually happens. That should create no objects and parameter passing is really fast in modern JS engines.
• consider dropping the function parameter feature from probe if it does not provide expected performance benefits (throw or warn if function parameter is passed).

log.info(() => `releasing ${id}`) // old, expensive inline function with variable capture
log.info(`releasing `, id) // new, fast parameter passing, string concatenation happens only of logging enabled.

Hi,

This issue is related to visgl/deck.gl#2241. Please, try to make asciify-image conditional in order to avoid to bring the +90 dependencies into the project.

Thanks,

Carlos.

The code added for #34 is not compatible with vite.js:

The problem is that vite will move all imports to the top of the file, so it's moved outside of the try/catch block.
While this is probably also a bug in vite.js, it's also a bit questionable to depend on the behaviour of other bundlers.

We require a workaround described in visgl/deck.gl#6134 (comment).