Skip to content
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
5 changes: 5 additions & 0 deletions .changeset/quiet-meta-identifier-refresh.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'posthog-js': patch
---

fix(browser): stop parsing the whole persistence entry on every capture() to refresh Meta identifiers. This made Interaction to Next Paint slower since 1.426.0, most of all on mobile and for projects with many feature flags.
107 changes: 107 additions & 0 deletions packages/browser/src/__tests__/capture-hot-path.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
import { uuidv7 } from '@posthog/browser-common/utils/uuidv7'
import { defaultPostHog } from './helpers/posthog-instance'
import { PERSISTENCE_FACEBOOK_CLICK_ID } from '../constants'
import type { PostHogConfig } from '../types'

// capture() runs inside click handlers, so its work adds directly to Interaction to Next Paint.
// Parsing the whole persistence entry on each capture made INP regress (#5428).
// These tests count the work instead of timing it, so a slow CI runner cannot make them flaky.

const ACTIVE_FEATURE_FLAGS = Array.from({ length: 500 }, (_, index) => `feature-flag-number-${index}`)

const persistenceEntryParses = (parseSpy: { mock: { calls: unknown[][] } }) =>
parseSpy.mock.calls.filter(([text]) => typeof text === 'string' && text.includes(ACTIVE_FEATURE_FLAGS[0]))

const setupWithManyFlags = (config: Partial<PostHogConfig>) => {
const token = uuidv7()
const posthog = defaultPostHog().init(token, config, token)!
posthog.persistence!.register({ $active_feature_flags: ACTIVE_FEATURE_FLAGS })
posthog.capture('warm up')
return { posthog, storageKey: `ph_${token}_posthog` }
}

describe('capture() hot path', () => {
afterEach(() => {
vi.restoreAllMocks()
localStorage.clear()
sessionStorage.clear()
})

it.each<[PostHogConfig['persistence'], boolean, boolean]>([
['localStorage+cookie', false, false],
['localStorage+cookie', true, false],
['localStorage+cookie', false, true],
['localStorage+cookie', true, true],
['localStorage', false, false],
['localStorage', false, true],
['sessionStorage', false, false],
['sessionStorage', false, true],
])(
'does not parse the persistence entry (persistence: %s, cookieWinsOnConflict: %s, stored $fbc: %s)',
(persistence, cookieWinsOnConflict, hasStoredFbc) => {
const { posthog } = setupWithManyFlags({ persistence, cookieWinsOnConflict })
if (hasStoredFbc) {
posthog.persistence!.register({
[PERSISTENCE_FACEBOOK_CLICK_ID]: { value: 'fb.1.1700000000000.click', delivered: true },
})
}

const parseSpy = vi.spyOn(JSON, 'parse')
for (let i = 0; i < 5; i++) {
posthog.capture('$autocapture')
}

expect(persistenceEntryParses(parseSpy)).toHaveLength(0)
}
)

it('adopts the $fbc that another tab wrote, then stops parsing the persistence entry', () => {
const { posthog, storageKey } = setupWithManyFlags({ persistence: 'localStorage' })
const siblingEntry = JSON.parse(localStorage.getItem(storageKey)!)
siblingEntry[PERSISTENCE_FACEBOOK_CLICK_ID] = { value: 'fb.1.1700000000000.sibling', delivered: true }
localStorage.setItem(storageKey, JSON.stringify(siblingEntry))

posthog.capture('$autocapture')
const parseSpy = vi.spyOn(JSON, 'parse')
posthog.capture('$autocapture')

expect(posthog.persistence!.props[PERSISTENCE_FACEBOOK_CLICK_ID]).toEqual(
siblingEntry[PERSISTENCE_FACEBOOK_CLICK_ID]
)
expect(persistenceEntryParses(parseSpy)).toHaveLength(0)
})

it('adopts the $fbc that another tab wrote while a refresh was parsing', () => {
const { posthog, storageKey } = setupWithManyFlags({ persistence: 'localStorage' })
const persistence = posthog.persistence!
const writeSiblingFbc = (value: string) => {
const siblingEntry = JSON.parse(localStorage.getItem(storageKey)!)
siblingEntry[PERSISTENCE_FACEBOOK_CLICK_ID] = { value, delivered: true }
localStorage.setItem(storageKey, JSON.stringify(siblingEntry))
}
writeSiblingFbc('fb.1.1700000000000.first')
const refreshKeys = persistence['_refreshKeys'].bind(persistence)
vi.spyOn(persistence as any, '_refreshKeys').mockImplementationOnce((keys) => {
refreshKeys(keys as string[])
writeSiblingFbc('fb.1.1700000000000.during-parse')
})

persistence.refreshMetaIdentifiers()
persistence.refreshMetaIdentifiers()

expect(persistence.props[PERSISTENCE_FACEBOOK_CLICK_ID]).toEqual({
value: 'fb.1.1700000000000.during-parse',
delivered: true,
})
})

it('does not read the persistence entry while persistence is disabled', () => {
const { posthog, storageKey } = setupWithManyFlags({ persistence: 'localStorage' })
posthog.set_config({ disable_persistence: true })

const getItemSpy = vi.spyOn(Storage.prototype, 'getItem')
posthog.capture('$autocapture')

expect(getItemSpy.mock.calls.filter(([key]) => key === storageKey)).toHaveLength(0)
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -819,7 +819,7 @@ const collectPostHogPersistenceMutationBoundaryIssues = (): string[] => {
'set_event_timer',
'remove_event_timer',
'set_property',
'refreshKey',
'_refreshKeys',
])

const visit = (node: ts.Node) => {
Expand Down
6 changes: 3 additions & 3 deletions packages/browser/src/posthog-core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1654,7 +1654,7 @@ export class PostHog implements PostHogInterface {
}

const channel = this._facebookClickIdChannel
this.persistence.refreshKey(channel.persistenceKey)
this.persistence.refreshMetaIdentifiers()
const stored = this._getPersistedMetaIdentifier(channel)

// An unset clears the store for this event only: the cookie stays, so a later event reads it again.
Expand Down Expand Up @@ -1703,7 +1703,7 @@ export class PostHog implements PostHogInterface {
}

const channel = this._facebookBrowserIdChannel
this.persistence.refreshKey(channel.persistenceKey)
this.persistence.refreshMetaIdentifiers()
const stored = this._getPersistedMetaIdentifier(channel)

// As for $fbc, an unset applies to this event only.
Expand Down Expand Up @@ -1732,7 +1732,7 @@ export class PostHog implements PostHogInterface {
if (!this.persistence) {
return
}
this.persistence.refreshKey(channel.persistenceKey)
this.persistence.refreshMetaIdentifiers()
const stored = this._getPersistedMetaIdentifier(channel)
if (stored?.value === value && !stored.delivered) {
channel.register(value, true)
Expand Down
86 changes: 69 additions & 17 deletions packages/browser/src/posthog-persistence.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ const GROUP_FRESHNESS_KEY: Partial<Record<PersistenceStorageGroup, string>> = {
surveys: SURVEYS_LOADED_AT,
}

import { isArray, isNull, isNumber, isUndefined } from '@posthog/core'
import { isArray, isNull, isNumber, isString, isUndefined } from '@posthog/core'
import {
getCampaignParams,
getInitialPersonPropsFromInfo,
Expand Down Expand Up @@ -255,6 +255,8 @@ export class PostHogPersistence {
private _localIdentityChangePending = false
private _crossTabFeatureFlagIdentityMismatch = false
private _metaIdentifierChangePending = new Set<string>()
// Storage as it was when the Meta identifiers in `props` last matched it.
private _metaIdentifierStorageSnapshot: string | undefined
private readonly _crossTabFeatureFlagHandlers = new Set<() => void>()
private _onStorage?: (event: StorageEvent) => void

Expand Down Expand Up @@ -1219,28 +1221,72 @@ export class PostHogPersistence {
* in-memory writes that haven't yet been debounced to storage.
*/
refreshKey(prop: string): void {
this._refreshKeys([prop])
}

// Reads the main entry at most once, however many keys are refreshed.
private _refreshKeys(props: readonly string[]): void {
if (this._disabled) {
return
}
if (this._metaIdentifierChangePending.has(prop)) {
return
}
const group = this._splitStorage ? getPersistenceKeyPolicy(prop)?.storageGroup : undefined
const entry = group ? localStore._parse(this._groupEntryName(group)) : this._storage._parse(this._name)
if (entry && prop in entry) {
this._setProp(prop, entry[prop], false)
return
let mainEntry: Properties | null | undefined
let mainEntryRead = false
const readMainEntry = () => {
if (!mainEntryRead) {
mainEntry = this._storage._parse(this._name)
mainEntryRead = true
}
return mainEntry
}
// A grouped key that has not migrated yet still lives in the main blob;
// check there once before concluding a sibling removed it.
if (group) {
const mainEntry = this._storage._parse(this._name)
if (mainEntry && prop in mainEntry) {
this._setProp(prop, mainEntry[prop], false)
props.forEach((prop) => {
if (this._metaIdentifierChangePending.has(prop)) {
return
}
const group = this._splitStorage ? getPersistenceKeyPolicy(prop)?.storageGroup : undefined
const entry = group ? localStore._parse(this._groupEntryName(group)) : readMainEntry()
if (entry && prop in entry) {
this._setProp(prop, entry[prop], false)
return
}
// A grouped key that has not migrated yet still lives in the main blob;
// check there once before concluding a sibling removed it.
if (group) {
const groupMainEntry = readMainEntry()
if (groupMainEntry && prop in groupMainEntry) {
this._setProp(prop, groupMainEntry[prop], false)
return
}
}
this._deleteProp(prop, false)
})
}

/**
* Refresh the Meta identifier keys from storage, so a sibling tab's update is not lost.
* capture() calls this on every event, so it parses storage only when storage has changed
* since the identifiers last matched it. A full parse on each click made INP regress.
*/
refreshMetaIdentifiers(): void {
if (this._disabled) {
return
}
const snapshot = this._storageSnapshot()
Comment thread
pauldambra marked this conversation as resolved.
Resolved
if (!isUndefined(snapshot) && snapshot === this._metaIdentifierStorageSnapshot) {
return
}
this._deleteProp(prop, false)
this._refreshKeys(META_IDENTIFIER_PERSISTENCE_KEYS)
// Remember the text read before the parse, so a sibling write that lands after it is parsed next time.
this._rememberMetaIdentifierStorageSnapshot(snapshot)
}

// A pending local change is not in storage yet, so storage does not match `props` until it is written.
private _rememberMetaIdentifierStorageSnapshot(snapshot = this._storageSnapshot()): void {
this._metaIdentifierStorageSnapshot = this._metaIdentifierChangePending.size === 0 ? snapshot : undefined
}

private _storageSnapshot(): string | undefined {
const snapshot = this._storage._rawText ? this._storage._rawText(this._name) : this._storage._get(this._name)
return isString(snapshot) ? snapshot : undefined
}

/**
Expand Down Expand Up @@ -1303,7 +1349,7 @@ export class PostHogPersistence {
!this._config.cookieWinsOnConflict ||
this._config.persistence.toLowerCase() !== 'localstorage+cookie'
) {
META_IDENTIFIER_PERSISTENCE_KEYS.forEach((key) => this.refreshKey(key))
this.refreshMetaIdentifiers()
}
}

Expand Down Expand Up @@ -1337,6 +1383,9 @@ export class PostHogPersistence {
this._localIdentityChangePending = false
this._metaIdentifierChangePending.clear()
}
if (writeResult === 'written') {
this._rememberMetaIdentifierStorageSnapshot()
}
if (crossTabPropertiesChanged) {
this._crossTabFeatureFlagHandlers.forEach((handler) => handler())
}
Expand Down Expand Up @@ -1368,6 +1417,9 @@ export class PostHogPersistence {
}
})
}
if (mainWriteResult === 'written') {
this._rememberMetaIdentifierStorageSnapshot()
}
for (const group of PERSISTENCE_STORAGE_GROUPS) {
const groupProps = groups[group]
// Don't materialize an entry just to hold `{}`: skip a group that is
Expand Down
7 changes: 7 additions & 0 deletions packages/browser/src/storage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -415,6 +415,13 @@ export const createLocalPlusCookieStore = (

return {
...localStore,
_rawText: function (name) {
return [
cookieStore._get(name),
cookieStore._get(getCookiePersistedPropertiesMetadataName(name)),
localStore._get(name),
].join('\u0000')
},
_parse: function (name) {
try {
let cookieValue: string | undefined
Expand Down
3 changes: 3 additions & 0 deletions packages/browser/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,9 @@ export interface PersistentStore {
_error: (error: any) => void
_parse: (name: string) => any
_get: (name: string) => any
// The raw text that `_parse` reads, for stores whose `_get` alone does not cover it.
// Equal snapshots mean `_parse` would return the same value.
_rawText?: (name: string) => string
// Returns whether the write was accepted without throwing. Backends that
// swallow quota/serialization errors return false so callers can avoid
// caching a write that never landed. This is best-effort, not a durable
Expand Down
5 changes: 5 additions & 0 deletions packages/browser/terser-mangled-names.json
Original file line number Diff line number Diff line change
Expand Up @@ -549,6 +549,7 @@
"_mergeWindowBack",
"_messagesEndRef",
"_metaIdentifierChangePending",
"_metaIdentifierStorageSnapshot",
"_metricsUrl",
"_migrateFromLegacyPersistence",
"_minimumDuration",
Expand Down Expand Up @@ -691,6 +692,7 @@
"_random",
"_rateCapWindowMs",
"_rateLimiter",
"_rawText",
"_read",
"_readLegacyFromRawStorage",
"_readMetaCookie",
Expand All @@ -712,6 +714,7 @@
"_refreshIfDue",
"_refreshInterval",
"_refreshIntervalMs",
"_refreshKeys",
"_refreshSessionIdFromStorage",
"_regionCache",
"_register",
Expand All @@ -728,6 +731,7 @@
"_reloadingHandlers",
"_rememberCrossTabStorageFingerprint",
"_rememberCurrentCookieProperties",
"_rememberMetaIdentifierStorageSnapshot",
"_remoteConfig",
"_remoteConfigLoader",
"_remoteConfigSubscription",
Expand Down Expand Up @@ -909,6 +913,7 @@
"_storage",
"_storageKey",
"_storageMigrationInProgress",
"_storageSnapshot",
"_storeMetaIdentifier",
"_storedConsent",
"_storesIdentityInCookie",
Expand Down
Loading