Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Eliminate the detection time window #1640

Merged
merged 1 commit into from
Nov 13, 2024
Merged

Eliminate the detection time window #1640

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 1 addition & 17 deletions plugins/optimization-detective/detect.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,8 +232,6 @@ function extendElementData( xpath, properties ) {
* Detects the LCP element, loaded images, client viewport and store for future optimizations.
*
* @param {Object} args Args.
* @param {number} args.serveTime The serve time of the page in milliseconds from PHP via `microtime( true ) * 1000`.
* @param {number} args.detectionTimeWindow The number of milliseconds between now and when the page was first generated in which detection should proceed.
* @param {string[]} args.extensionModuleUrls URLs for extension script modules to import.
* @param {number} args.minViewportAspectRatio Minimum aspect ratio allowed for the viewport.
* @param {number} args.maxViewportAspectRatio Maximum aspect ratio allowed for the viewport.
Expand All @@ -248,8 +246,6 @@ function extendElementData( xpath, properties ) {
* @param {Object} [args.urlMetricGroupCollection] URL Metric group collection, when in debug mode.
*/
export default async function detect( {
serveTime,
detectionTimeWindow,
minViewportAspectRatio,
maxViewportAspectRatio,
isDebug,
Expand All @@ -263,22 +259,10 @@ export default async function detect( {
webVitalsLibrarySrc,
urlMetricGroupCollection,
} ) {
const currentTime = getCurrentTime();

if ( isDebug ) {
log( 'Stored URL Metric group collection:', urlMetricGroupCollection );
}

// Abort running detection logic if it was served in a cached page.
if ( currentTime - serveTime > detectionTimeWindow ) {
if ( isDebug ) {
warn(
'Aborted detection due to being outside detection time window.'
);
}
return;
}

// Abort if the current viewport is not among those which need URL Metrics.
if ( ! isViewportNeeded( win.innerWidth, urlMetricGroupStatuses ) ) {
if ( isDebug ) {
Expand Down Expand Up @@ -330,7 +314,7 @@ export default async function detect( {
// As an alternative to this, the od_print_detection_script() function can short-circuit if the
// od_is_url_metric_storage_locked() function returns true. However, the downside with that is page caching could
// result in metrics missed from being gathered when a user navigates around a site and primes the page cache.
if ( isStorageLocked( currentTime, storageLockTTL ) ) {
if ( isStorageLocked( getCurrentTime(), storageLockTTL ) ) {
if ( isDebug ) {
warn( 'Aborted detection due to storage being locked.' );
}
Expand Down
17 changes: 0 additions & 17 deletions plugins/optimization-detective/detection.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,21 +20,6 @@
* @param OD_URL_Metric_Group_Collection $group_collection URL Metric group collection.
*/
function od_get_detection_script( string $slug, OD_URL_Metric_Group_Collection $group_collection ): string {
/**
* Filters the time window between serve time and run time in which loading detection is allowed to run.
*
* This is the allowance of milliseconds between when the page was first generated (and perhaps cached) and when the
* detect function on the page is allowed to perform its detection logic and submit the request to store the results.
* This avoids situations in which there is missing URL Metrics in which case a site with page caching which
* also has a lot of traffic could result in a cache stampede.
*
* @since 0.1.0
* @todo The value should probably be something like the 99th percentile of Time To Last Byte (TTLB) for WordPress sites in CrUX.
*
* @param int $detection_time_window Detection time window in milliseconds.
*/
$detection_time_window = apply_filters( 'od_detection_time_window', 5000 );

$web_vitals_lib_data = require __DIR__ . '/build/web-vitals.asset.php';
$web_vitals_lib_src = add_query_arg( 'ver', $web_vitals_lib_data['version'], plugin_dir_url( __FILE__ ) . 'build/web-vitals.js' );

Expand All @@ -49,8 +34,6 @@ function od_get_detection_script( string $slug, OD_URL_Metric_Group_Collection $

$current_url = od_get_current_url();
$detect_args = array(
'serveTime' => microtime( true ) * 1000, // In milliseconds for comparison with `Date.now()` in JavaScript.
'detectionTimeWindow' => $detection_time_window,
'minViewportAspectRatio' => od_get_minimum_viewport_aspect_ratio(),
'maxViewportAspectRatio' => od_get_maximum_viewport_aspect_ratio(),
'isDebug' => WP_DEBUG,
Expand Down
4 changes: 0 additions & 4 deletions plugins/optimization-detective/readme.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,10 +132,6 @@ Filters the freshness age (TTL) for a given URL Metric. The freshness TTL must b
add_filter( 'od_url_metric_freshness_ttl', '__return_zero' );
`

**Filter:** `od_detection_time_window` (default: 5 seconds)

Filters the time window between serve time and run time in which loading detection is allowed to run. This amount is the allowance between when the page was first generated (and perhaps cached) and when the detect function on the page is allowed to perform its detection logic and submit the request to store the results. This avoids situations in which there are missing URL Metrics in which case a site with page caching which also has a lot of traffic could result in a cache stampede.

**Filter:** `od_minimum_viewport_aspect_ratio` (default: 0.4)

Filters the minimum allowed viewport aspect ratio for URL Metrics.
Expand Down
8 changes: 0 additions & 8 deletions plugins/optimization-detective/tests/test-detection.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,12 @@ public function data_provider_od_get_detection_script(): array {
'unfiltered' => array(
'set_up' => static function (): void {},
'expected_exports' => array(
'detectionTimeWindow' => 5000,
'storageLockTTL' => MINUTE_IN_SECONDS,
'extensionModuleUrls' => array(),
),
),
'filtered' => array(
'set_up' => static function (): void {
add_filter(
'od_detection_time_window',
static function (): int {
return 2500;
}
);
add_filter(
'od_url_metric_storage_lock_ttl',
static function (): int {
Expand All @@ -45,7 +38,6 @@ static function ( array $urls ): array {
);
},
'expected_exports' => array(
'detectionTimeWindow' => 2500,
'storageLockTTL' => HOUR_IN_SECONDS,
'extensionModuleUrls' => array( home_url( '/my-extension.js', 'https' ) ),
),
Expand Down
Loading