[Sticky] Number of paint calls is higher than expected

[krausest]

Benchmarking some of the frameworks yields a warning like "For framework reflex-dom and benchmark 01_run1k the number of paint calls is higher than expected. There were 3 paints though at most 2 are expected. Please consider re-running and check the results"

Looks like this happens when the remove icon is not preloaded and thus repaints occur once the glyph is loaded. Every benchmark must make sure that the following span in included:

<span class="preloadicon glyphicon glyphicon-remove" aria-hidden="true"></span>

[krausest]

See 2fda7e9 for vanilla-dom-js, 01b07c2 for pux.
Warnings are gone for these frameworks.

[krausest]

@alexfmpe I think the warning for reflex-dom is also due to that missing span. Can you please add it?

[krausest]

Looks good. No more warnings for those benchmarks. I'll leave the issue open for future reference.

[mindplay-dk]

Why is there a cap on the number of paint calls?

It appears to cause problems with the render scheduler in Fre - see this:

frejs/fre#271 (comment)

The code seems to suggest this is a warning and not an error?

js-framework-benchmark/webdriver-ts/src/forkedBenchmarkRunner.ts

Lines 206 to 209 in dc4e9bb

	if (paints.length>2) {
	warnings.push(`For framework ${framework.name} and benchmark ${benchmark.id} the number of paint calls is higher than expected. There were ${paints.length} paints though at most 2 are expected. Please consider re-running and check the results`);
	console.log(`For framework ${framework.name} and benchmark ${benchmark.id} the number of paint calls is higher than expected. There were ${paints.length} paints though at most 2 are expected. Please consider re-running and check the results`);
	}

Not sure if this warning matters, or whether it's avoidable with Fre.

It's an interesting new framework with a fairly novel approach to rendering and scheduling - it would be nice if we could get this onboard with the benchmarks, but I wonder if this type of scheduling makes it inherently difficult to benchmark. 🤔

[krausest]

This message mattered for older versions of chrome when more than a single paint event really meant that something didn't behave as expected. For newer chrome version this occurs fairly often, even for vanillajs like

# of paint events  2
duration to paint  134.088
duration to paint  137.612

The benchmark uses the last paint event to compute the duration for an operation. I took a look at fre, which reports 3 paint events for create rows:

# of paint events  3
paint  1.79
duration to paint  153.488
duration to paint  155.631

This looks OK to me (and is what I see in the timeline). I think I'll remove the warning someday. Please ignore the warnings for the time being.

BTW the fre implementation isn't correctly keyed regarding create rows and swap rows:

npm run isKeyed -- --headless keyed/fre2
...
Frameworks that will be checked fre2-v2.3.0-keyed
Keyed test for swap failed. Expected at least 1 added and 1 removed TR, but there were 0 added and 0 removed
Keyed test for create rows failed. Expected that 1000 TRs should be removed and added, but there were 0 added TRs and 0 were removed
fre2-v2.3.0-keyed is non-keyed for 'run benchmark' and keyed for 'remove row benchmark' and non-keyed for 'swap rows benchmark' . It'll appear as non-keyed in the results

Looks like fre doesn't swap the rows which is a requirement for a keyed implementation and it doesn't replace the rows but re-uses them for create rows. This is fine for a non-keyed implementation but not so for keyed.

[yisar]

@krausest

fre-vinvalid-keyed is keyed for 'run benchmark' and keyed for 'remove row benchmark' and keyed for 'swap rows benchmark' . It'll appear as keyed in the results

Very strange, because it seems normal on my computer. I'll reopen another issue.

#932

If I switch internal scheduling to micro tasks, these warnings will disappear, but I don't want to do so. Because Time slicing is fre's default option.