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.

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

Fix performance measures crashing the app #54858

Merged
merged 8 commits into from
Sep 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
142 changes: 91 additions & 51 deletions packages/next/src/client/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -439,88 +439,128 @@ function Head({ callback }: { callback: () => void }): null {
return null
}

const performanceMarks = {
navigationStart: 'navigationStart',
beforeRender: 'beforeRender',
afterRender: 'afterRender',
afterHydrate: 'afterHydrate',
routeChange: 'routeChange',
} as const

const performanceMeasures = {
hydration: 'Next.js-hydration',
beforeHydration: 'Next.js-before-hydration',
routeChangeToRender: 'Next.js-route-change-to-render',
render: 'Next.js-render',
} as const

let reactRoot: any = null
// On initial render a hydrate should always happen
let shouldHydrate: boolean = true

function clearMarks(): void {
;['beforeRender', 'afterHydrate', 'afterRender', 'routeChange'].forEach(
(mark) => performance.clearMarks(mark)
)
;[
performanceMarks.beforeRender,
performanceMarks.afterHydrate,
performanceMarks.afterRender,
performanceMarks.routeChange,
].forEach((mark) => performance.clearMarks(mark))
}

function markHydrateComplete(): void {
if (!ST) return

performance.mark('afterHydrate') // mark end of hydration
performance.mark(performanceMarks.afterHydrate) // mark end of hydration

const beforeHydrationMeasure = performance.measure(
'Next.js-before-hydration',
'navigationStart',
'beforeRender'
)
const hasBeforeRenderMark = performance.getEntriesByName(
performanceMarks.beforeRender,
'mark'
).length
if (hasBeforeRenderMark) {
const beforeHydrationMeasure = performance.measure(
performanceMeasures.beforeHydration,
performanceMarks.navigationStart,
performanceMarks.beforeRender
)

const hydrationMeasure = performance.measure(
'Next.js-hydration',
'beforeRender',
'afterHydrate'
)
const hydrationMeasure = performance.measure(
performanceMeasures.hydration,
performanceMarks.beforeRender,
performanceMarks.afterHydrate
)

if (
process.env.NODE_ENV === 'development' &&
// Old versions of Safari don't return `PerformanceMeasure`s from `performance.measure()`
beforeHydrationMeasure !== undefined &&
hydrationMeasure !== undefined
) {
tracer
.startSpan('navigation-to-hydration', {
startTime: performance.timeOrigin + beforeHydrationMeasure.startTime,
attributes: {
pathname: location.pathname,
query: location.search,
},
})
.end(
performance.timeOrigin +
hydrationMeasure.startTime +
hydrationMeasure.duration
)
if (
process.env.NODE_ENV === 'development' &&
// Old versions of Safari don't return `PerformanceMeasure`s from `performance.measure()`
beforeHydrationMeasure !== undefined &&
hydrationMeasure !== undefined
) {
tracer
.startSpan('navigation-to-hydration', {
startTime: performance.timeOrigin + beforeHydrationMeasure.startTime,
attributes: {
pathname: location.pathname,
query: location.search,
},
})
.end(
performance.timeOrigin +
hydrationMeasure.startTime +
hydrationMeasure.duration
)
}
}

if (onPerfEntry) {
performance.getEntriesByName('Next.js-hydration').forEach(onPerfEntry)
performance
.getEntriesByName(performanceMeasures.hydration)
.forEach(onPerfEntry)
}
clearMarks()
}

function markRenderComplete(): void {
if (!ST) return

performance.mark('afterRender') // mark end of render
performance.mark(performanceMarks.afterRender) // mark end of render
const navStartEntries: PerformanceEntryList = performance.getEntriesByName(
'routeChange',
performanceMarks.routeChange,
'mark'
)

if (!navStartEntries.length) return

performance.measure(
'Next.js-route-change-to-render',
navStartEntries[0].name,
'beforeRender'
)
performance.measure('Next.js-render', 'beforeRender', 'afterRender')
if (onPerfEntry) {
performance.getEntriesByName('Next.js-render').forEach(onPerfEntry)
performance
.getEntriesByName('Next.js-route-change-to-render')
.forEach(onPerfEntry)
const hasBeforeRenderMark = performance.getEntriesByName(
performanceMarks.beforeRender,
'mark'
).length

if (hasBeforeRenderMark) {
performance.measure(
performanceMeasures.routeChangeToRender,
navStartEntries[0].name,
performanceMarks.beforeRender
)
performance.measure(
performanceMeasures.render,
performanceMarks.beforeRender,
performanceMarks.afterRender
)
if (onPerfEntry) {
performance
.getEntriesByName(performanceMeasures.render)
.forEach(onPerfEntry)
performance
.getEntriesByName(performanceMeasures.routeChangeToRender)
.forEach(onPerfEntry)
}
}

clearMarks()
;['Next.js-route-change-to-render', 'Next.js-render'].forEach((measure) =>
performance.clearMeasures(measure)
)
;[
performanceMeasures.routeChangeToRender,
performanceMeasures.render,
].forEach((measure) => performance.clearMeasures(measure))
}

function renderReactElement(
Expand All @@ -529,7 +569,7 @@ function renderReactElement(
): void {
// mark start of hydrate/render
if (ST) {
performance.mark('beforeRender')
performance.mark(performanceMarks.beforeRender)
}

const reactEl = fn(shouldHydrate ? markHydrateComplete : markRenderComplete)
Expand Down
17 changes: 17 additions & 0 deletions test/e2e/pages-performance-mark/index.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { createNextDescribe } from 'e2e-utils'

// This test case doesn't indicate rendering duplicate head in _document is valid,
// but it's a way to reproduce the performance mark crashing.
createNextDescribe(
'pages performance mark',
{
files: __dirname,
skipDeployment: true,
},
({ next }) => {
it('should render the page correctly without crashing with performance mark', async () => {
const browser = await next.browser('/')
expect(await browser.elementByCss('h1').text()).toBe('home')
})
}
)
6 changes: 6 additions & 0 deletions test/e2e/pages-performance-mark/next.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
/** @type {import('next').NextConfig} */
const nextConfig = {
reactStrictMode: true,
}

module.exports = nextConfig
14 changes: 14 additions & 0 deletions test/e2e/pages-performance-mark/pages/_document.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { Html, Head, Main, NextScript } from 'next/document'

export default function Document() {
return (
<Html>
<Head />
<Head />
<body>
<Main />
<NextScript />
</body>
</Html>
)
}
3 changes: 3 additions & 0 deletions test/e2e/pages-performance-mark/pages/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Home() {
return <h1>home</h1>
}