Allow raw marker table's startTime and endTime columns to be Float64Array

[mstange]

Main | Deploy preview

The "raw" vs "derived" split for markers is a bit different than for the other tables, but regardless, it turns out we can just make the raw version take typed arrays without much trouble. We just needed to add the builder.

[codecov]

Codecov Report

❌ Patch coverage is 84.74576% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.62%. Comparing base (1373a9d) to head (0c507ac).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
src/profile-logic/process-profile.ts	72.72%	6 Missing ⚠️
src/utils/typed-arrays.ts	0.00%	2 Missing ⚠️
src/profile-logic/data-structures.ts	83.33%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff            @@
##             main    #6169    +/-   ##
========================================
  Coverage   83.62%   83.62%            
========================================
  Files         345      346     +1     
  Lines       37051    37165   +114     
  Branches    10270    10425   +155     
========================================
+ Hits        30983    31080    +97     
- Misses       5640     5657    +17     
  Partials      428      428            

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[canova]

Thanks! I think maybeAddMetricMarker comment is a real bug, but others are mostly questions.

[canova]

Now it looks like on every marker addition we are cloning the whole marker table (we are in the maybeAddMetricMarker function here). Can we call this before and maybe pass as a param?

Also checked the other getRawMarkerTableBuilderFromExisting calls but they look good to me.

[mstange]

Oops indeed! Fixed.

[canova]

Ah weird, thanks for the explanation!

Do we have checks like num === null in some places though? Can it be a problem?

[mstange]

For marker startTime and endTime, the only null checks I can find are checks in the form of ensureExists:

profiler/src/profile-logic/profile-data.ts

Lines 1496 to 1515 in bd2a5cf

	switch (markerPhase) {
	case INSTANT:
	case INTERVAL_START: {
	const startTime = ensureExists(maybeStartTime);
	result.start = Math.min(result.start, startTime);
	result.end = Math.max(result.end, startTime + interval);
	break;
	}
	case INTERVAL_END: {
	const endTime = ensureExists(maybeEndTime);
	result.start = Math.min(result.start, endTime);
	result.end = Math.max(result.end, endTime + interval);
	break;
	}
	case INTERVAL: {
	const startTime = ensureExists(maybeStartTime);
	const endTime = ensureExists(maybeEndTime);

profiler/src/profile-logic/marker-data.ts

Lines 682 to 683 in bd2a5cf

	const endStartTime = ensureExists(maybeStartTime, ensureMessage);
	const endEndTime = ensureExists(maybeEndTime, ensureMessage);

[mstange]

So this code would already be throwing exceptions if it encountered nulls in places where the phase wants a non-null value.

[canova]

Ah that makes sense, thanks for checking!

[canova]

Thanks!