[MM-63794] Remove the Redux Selector telemetry (#30794)

This commit is contained in:
M-ZubairAhmed 2025-04-29 16:51:43 +05:30 committed by GitHub
parent df3560ed9c
commit 4f80dda772
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 2 additions and 606 deletions

View file

@ -3,7 +3,6 @@
import {Client4} from 'mattermost-redux/client';
import {Preferences} from 'mattermost-redux/constants';
import {getSortedTrackedSelectors} from 'mattermost-redux/selectors/create_selector';
import {getConfig, isPerformanceDebuggingEnabled} from 'mattermost-redux/selectors/entities/general';
import {getBool} from 'mattermost-redux/selectors/entities/preferences';
@ -172,30 +171,6 @@ export function trackPluginInitialization(plugins) {
});
}
export function trackSelectorMetrics() {
setTimeout(() => {
if (!shouldTrackPerformance()) {
return;
}
const selectors = getSortedTrackedSelectors();
const filteredSelectors = selectors.filter((selector) => selector.calls > 5);
trackEvent('performance', 'least_effective_selectors', {
after: 'one_minute',
first: filteredSelectors[0]?.name || '',
first_effectiveness: filteredSelectors[0]?.effectiveness,
first_recomputations: filteredSelectors[0]?.recomputations,
second: filteredSelectors[1]?.name || '',
second_effectiveness: filteredSelectors[1]?.effectiveness,
second_recomputations: filteredSelectors[1]?.recomputations,
third: filteredSelectors[2]?.name || '',
third_effectiveness: filteredSelectors[2]?.effectiveness,
third_recomputations: filteredSelectors[2]?.recomputations,
});
}, 60000);
}
let requestCount = 0;
const requestCountAtMark = {};
let requestObserver;

View file

@ -14,7 +14,7 @@ import {setUrl} from 'mattermost-redux/actions/general';
import {Client4} from 'mattermost-redux/client';
import {Preferences} from 'mattermost-redux/constants';
import {measurePageLoadTelemetry, temporarilySetPageLoadContext, trackEvent, trackSelectorMetrics} from 'actions/telemetry_actions.jsx';
import {measurePageLoadTelemetry, temporarilySetPageLoadContext, trackEvent} from 'actions/telemetry_actions.jsx';
import BrowserStore from 'stores/browser_store';
import {makeAsyncComponent, makeAsyncPluggableComponent} from 'components/async_load';
@ -360,7 +360,6 @@ export default class Root extends React.PureComponent<Props, State> {
this.initiateMeRequests();
measurePageLoadTelemetry();
trackSelectorMetrics();
// Force logout of all tabs if one tab is logged out
window.addEventListener('storage', this.handleLogoutLoginSignal);

View file

@ -3,26 +3,6 @@
/* eslint-disable */
// Generates a RFC-4122 version 4 compliant globally unique identifier.
export function generateId() {
// implementation taken from http://stackoverflow.com/a/2117523
let id = 'xxxxxxxx-xxxx-4xxx-yxxx-xxxxxxxxxxxx';
id = id.replace(/[xy]/g, (c) => {
const r = Math.floor(Math.random() * 16);
let v;
if (c === 'x') {
v = r;
} else {
// eslint-disable-next-line no-mixed-operators
v = r & 0x3 | 0x8;
}
return v.toString(16);
});
return id;
}
function defaultEqualityCheck(a, b) {
return a === b;
}
@ -43,7 +23,7 @@ function areArgumentsShallowlyEqual(equalityCheck, prev, next) {
return true;
}
export function defaultMemoize(func, measure, equalityCheck = defaultEqualityCheck) {
export function defaultMemoize(func, equalityCheck = defaultEqualityCheck) {
let lastArgs = null
let lastResult = null
// we reference arguments instead of spreading them for performance reasons
@ -53,9 +33,6 @@ export function defaultMemoize(func, measure, equalityCheck = defaultEqualityChe
lastResult = func.apply(null, arguments)
}
if (measure) {
measure();
}
lastArgs = arguments
return lastResult
}
@ -77,25 +54,16 @@ function getDependencies(funcs) {
return dependencies;
}
const trackedSelectors = {};
export function createSelectorCreator(memoize, ...memoizeOptions) {
return (name, ...funcs) => {
const id = generateId();
let recomputations = 0;
let calls = 0;
const resultFunc = funcs.pop();
const dependencies = getDependencies(funcs);
const memoizedResultFunc = memoize(
function() {
recomputations++;
trackedSelectors[id].recomputations++;
// apply arguments instead of spreading for performance.
return resultFunc?.apply(null, arguments);
},
null,
...memoizeOptions,
);
@ -111,23 +79,10 @@ export function createSelectorCreator(memoize, ...memoizeOptions) {
// apply arguments instead of spreading for performance.
return memoizedResultFunc.apply(null, params);
},
() => {
calls++;
trackedSelectors[id].calls++;
});
selector.resultFunc = resultFunc;
selector.dependencies = dependencies;
selector.recomputations = () => recomputations;
selector.resetRecomputations = () => recomputations = 0;
trackedSelectors[id] = {
id,
name,
calls: 0,
recomputations: 0,
};
return selector;
};
@ -153,52 +108,3 @@ export function createStructuredSelector(selectors, selectorCreator = createSele
},
);
}
// resetTrackedSelectors resets all the measurements for memoization effectiveness.
function resetTrackedSelectors() {
Object.values(trackedSelectors).forEach((selector) => {
selector.calls = 0;
selector.recomputations = 0;
});
}
// getSortedTrackedSelectors returns an array, sorted by effectivness, containing mesaurement data on all tracked selectors.
export function getSortedTrackedSelectors() {
let selectors = Object.values(trackedSelectors);
// Filter out any selector not called
selectors = selectors.filter(selector => selector.calls > 0);
const selectorsData = selectors.map((selector) => ({name: selector.name, effectiveness: effectiveness(selector), recomputations: selector.recomputations, calls: selector.calls}));
selectorsData.sort((a, b) => {
// Sort effectiveness ascending
if (a.effectiveness !== b.effectiveness) {
return a.effectiveness - b.effectiveness;
}
// And everything else descending
if (a.recomputations !== b.recomputations) {
return b.recomputations - a.recomputations;
}
if (a.calls !== b.calls) {
return b.calls - a.calls;
}
return a.name.localeCompare(b.name);
});
return selectorsData;
}
function effectiveness(selector) {
return 100 - ((selector.recomputations / selector.calls) * 100);
}
// dumpTrackedSelectorsStatistics prints to console a table containing the measurement data on all tracked selectors.
function dumpTrackedSelectorsStatistics() {
const selectors = getSortedTrackedSelectors();
console.table(selectors); //eslint-disable-line no-console
}
window.dumpTrackedSelectorsStatistics = dumpTrackedSelectorsStatistics;
window.resetTrackedSelectors = resetTrackedSelectors;
window.getSortedTrackedSelectors = getSortedTrackedSelectors

View file

@ -239,241 +239,6 @@ describe('makeFilterPostsAndAddSeparators', () => {
'date-' + (today.getTime() + 1000),
]);
});
it('memoization', () => {
const filterPostsAndAddSeparators = makeFilterPostsAndAddSeparators();
const time = Date.now();
const today = new Date(time);
const tomorrow = new Date((24 * 60 * 60 * 1000) + today.getTime());
// Posts 7 hours apart so they should appear on multiple days
const initialPosts = {
1001: {id: '1001', create_at: time, type: ''},
1002: {id: '1002', create_at: time + 5, type: ''},
1003: {id: '1003', create_at: time + 10, type: ''},
1004: {id: '1004', create_at: tomorrow, type: ''},
1005: {id: '1005', create_at: tomorrow as any + 5, type: ''},
1006: {id: '1006', create_at: tomorrow as any + 10, type: Posts.POST_TYPES.JOIN_CHANNEL},
};
let state = {
entities: {
general: {
config: {},
},
posts: {
posts: initialPosts,
},
preferences: {
myPreferences: {
[getPreferenceKey(Preferences.CATEGORY_ADVANCED_SETTINGS, Preferences.ADVANCED_FILTER_JOIN_LEAVE)]: {
category: Preferences.CATEGORY_ADVANCED_SETTINGS,
name: Preferences.ADVANCED_FILTER_JOIN_LEAVE,
value: 'true',
},
},
},
users: {
currentUserId: '1234',
profiles: {
1234: {id: '1234', username: 'user'},
},
},
},
} as unknown as GlobalState;
let postIds = [
'1006',
'1004',
'1003',
'1001',
];
let lastViewedAt = initialPosts['1001'].create_at + 1;
let now = filterPostsAndAddSeparators(state, {postIds, lastViewedAt, indicateNewMessages: true});
expect(now).toEqual([
'1006',
'1004',
'date-' + tomorrow.getTime(),
'1003',
START_OF_NEW_MESSAGES + lastViewedAt,
'1001',
'date-' + today.getTime(),
]);
// No changes
let prev = now;
now = filterPostsAndAddSeparators(state, {postIds, lastViewedAt, indicateNewMessages: true});
expect(now).toBe(prev);
expect(now).toEqual([
'1006',
'1004',
'date-' + tomorrow.getTime(),
'1003',
START_OF_NEW_MESSAGES + lastViewedAt,
'1001',
'date-' + today.getTime(),
]);
// lastViewedAt changed slightly
lastViewedAt = initialPosts['1001'].create_at + 2;
prev = now;
now = filterPostsAndAddSeparators(state, {postIds, lastViewedAt, indicateNewMessages: true});
expect(now).not.toBe(prev);
expect(now).toEqual([
'1006',
'1004',
'date-' + tomorrow.getTime(),
'1003',
START_OF_NEW_MESSAGES + lastViewedAt,
'1001',
'date-' + today.getTime(),
]);
// lastViewedAt changed a lot
lastViewedAt = initialPosts['1003'].create_at + 1;
prev = now;
now = filterPostsAndAddSeparators(state, {postIds, lastViewedAt, indicateNewMessages: true});
expect(now).not.toBe(prev);
expect(now).toEqual([
'1006',
'1004',
START_OF_NEW_MESSAGES + lastViewedAt,
'date-' + tomorrow.getTime(),
'1003',
'1001',
'date-' + today.getTime(),
]);
prev = now;
now = filterPostsAndAddSeparators(state, {postIds, lastViewedAt, indicateNewMessages: true});
expect(now).toBe(prev);
expect(now).toEqual([
'1006',
'1004',
START_OF_NEW_MESSAGES + lastViewedAt,
'date-' + tomorrow.getTime(),
'1003',
'1001',
'date-' + today.getTime(),
]);
// postIds changed, but still shallowly equal
postIds = [...postIds];
prev = now;
now = filterPostsAndAddSeparators(state, {postIds, lastViewedAt, indicateNewMessages: true});
expect(now).toBe(prev);
expect(now).toEqual([
'1006',
'1004',
START_OF_NEW_MESSAGES + lastViewedAt,
'date-' + tomorrow.getTime(),
'1003',
'1001',
'date-' + today.getTime(),
]);
// Post changed, not in postIds
state = {
...state,
entities: {
...state.entities,
posts: {
...state.entities.posts,
posts: {
...state.entities.posts.posts,
1007: {id: '1007', create_at: 7 * 60 * 60 * 7 * 1000},
},
},
},
} as unknown as GlobalState;
prev = now;
now = filterPostsAndAddSeparators(state, {postIds, lastViewedAt, indicateNewMessages: true});
expect(now).toBe(prev);
expect(now).toEqual([
'1006',
'1004',
START_OF_NEW_MESSAGES + lastViewedAt,
'date-' + tomorrow.getTime(),
'1003',
'1001',
'date-' + today.getTime(),
]);
// Post changed, in postIds
state = {
...state,
entities: {
...state.entities,
posts: {
...state.entities.posts,
posts: {
...state.entities.posts.posts,
1006: {...state.entities.posts.posts['1006'], message: 'abcd'},
},
},
},
};
prev = now;
now = filterPostsAndAddSeparators(state, {postIds, lastViewedAt, indicateNewMessages: true});
expect(now).toBe(prev);
expect(now).toEqual([
'1006',
'1004',
START_OF_NEW_MESSAGES + lastViewedAt,
'date-' + tomorrow.getTime(),
'1003',
'1001',
'date-' + today.getTime(),
]);
// Filter changed
state = {
...state,
entities: {
...state.entities,
preferences: {
...state.entities.preferences,
myPreferences: {
...state.entities.preferences.myPreferences,
[getPreferenceKey(Preferences.CATEGORY_ADVANCED_SETTINGS, Preferences.ADVANCED_FILTER_JOIN_LEAVE)]: {
category: Preferences.CATEGORY_ADVANCED_SETTINGS,
name: Preferences.ADVANCED_FILTER_JOIN_LEAVE,
value: 'false',
},
} as unknown as GlobalState['entities']['preferences']['myPreferences'],
},
},
};
prev = now;
now = filterPostsAndAddSeparators(state, {postIds, lastViewedAt, indicateNewMessages: true});
expect(now).not.toBe(prev);
expect(now).toEqual([
'1004',
START_OF_NEW_MESSAGES + lastViewedAt,
'date-' + tomorrow.getTime(),
'1003',
'1001',
'date-' + today.getTime(),
]);
prev = now;
now = filterPostsAndAddSeparators(state, {postIds, lastViewedAt, indicateNewMessages: true});
expect(now).toBe(prev);
expect(now).toEqual([
'1004',
START_OF_NEW_MESSAGES + lastViewedAt,
'date-' + tomorrow.getTime(),
'1003',
'1001',
'date-' + today.getTime(),
]);
});
});
describe('makeCombineUserActivityPosts', () => {
@ -705,155 +470,6 @@ describe('makeCombineUserActivityPosts', () => {
expect(result).toHaveLength(2);
});
describe('memoization', () => {
const initialPostIds = ['post1', 'post2'];
const initialState = {
entities: {
posts: {
posts: {
post1: {id: 'post1', type: Posts.POST_TYPES.JOIN_CHANNEL},
post2: {id: 'post2', type: Posts.POST_TYPES.JOIN_CHANNEL},
},
},
},
} as unknown as GlobalState;
test('should not recalculate when nothing has changed', () => {
const combineUserActivityPosts = makeCombineUserActivityPosts();
expect(combineUserActivityPosts.recomputations()).toBe(0);
combineUserActivityPosts(initialState, initialPostIds);
expect(combineUserActivityPosts.recomputations()).toBe(1);
combineUserActivityPosts(initialState, initialPostIds);
expect(combineUserActivityPosts.recomputations()).toBe(1);
});
test('should recalculate when the post IDs change', () => {
const combineUserActivityPosts = makeCombineUserActivityPosts();
let postIds = initialPostIds;
combineUserActivityPosts(initialState, postIds);
expect(combineUserActivityPosts.recomputations()).toBe(1);
postIds = ['post1'];
combineUserActivityPosts(initialState, postIds);
expect(combineUserActivityPosts.recomputations()).toBe(2);
});
test('should not recalculate when an unrelated state change occurs', () => {
const combineUserActivityPosts = makeCombineUserActivityPosts();
let state = initialState;
combineUserActivityPosts(state, initialPostIds);
expect(combineUserActivityPosts.recomputations()).toBe(1);
state = {
...state,
entities: {
...state.entities,
posts: {
...state.entities.posts,
messagesHistory: {
messages: [],
index: {
post: 4,
comment: 3,
},
},
},
},
};
combineUserActivityPosts(state, initialPostIds);
expect(combineUserActivityPosts.recomputations()).toBe(1);
});
test('should not recalculate if an unrelated post changes', () => {
const combineUserActivityPosts = makeCombineUserActivityPosts();
let state = initialState;
const initialResult = combineUserActivityPosts(state, initialPostIds);
expect(combineUserActivityPosts.recomputations()).toBe(1);
// An unrelated post changed
state = {
...state,
entities: {
...state.entities,
posts: {
...state.entities.posts,
posts: {
...state.entities.posts.posts,
post3: TestHelper.getPostMock({id: 'post3'}),
},
},
},
};
const result = combineUserActivityPosts(state, initialPostIds);
// The selector didn't recalculate so the result didn't change
expect(combineUserActivityPosts.recomputations()).toBe(1);
expect(result).toBe(initialResult);
});
test('should return the same result when a post changes in a way that doesn\'t affect the result', () => {
const combineUserActivityPosts = makeCombineUserActivityPosts();
let state = initialState;
const initialResult = combineUserActivityPosts(state, initialPostIds);
expect(combineUserActivityPosts.recomputations()).toBe(1);
// One of the posts was updated, but post type didn't change
state = {
...state,
entities: {
...state.entities,
posts: {
...state.entities.posts,
posts: {
...state.entities.posts.posts,
post2: {...state.entities.posts.posts.post2, update_at: 1234},
},
},
},
};
let result = combineUserActivityPosts(state, initialPostIds);
// The selector recalculated but is still returning the same array
expect(combineUserActivityPosts.recomputations()).toBe(2);
expect(result).toBe(initialResult);
// One of the posts changed type
state = {
...state,
entities: {
...state.entities,
posts: {
...state.entities.posts,
posts: {
...state.entities.posts.posts,
post2: {...state.entities.posts.posts.post2, type: ''},
},
},
},
};
result = combineUserActivityPosts(state, initialPostIds);
// The selector recalculated, and the result changed
expect(combineUserActivityPosts.recomputations()).toBe(3);
expect(result).not.toBe(initialResult);
});
});
});
describe('isDateLine', () => {
@ -1052,106 +668,6 @@ describe('makeGenerateCombinedPost', () => {
metadata: {},
});
});
describe('memoization', () => {
const initialState = {
entities: {
posts: {
posts: {
post1: {id: 'post1'},
post2: {id: 'post2'},
},
},
},
} as unknown as GlobalState;
const initialCombinedId = 'user-activity-post1_post2';
test('should not recalculate when called twice with the same ID', () => {
const generateCombinedPost = makeGenerateCombinedPost();
expect((generateCombinedPost as any).recomputations()).toBe(0);
generateCombinedPost(initialState, initialCombinedId);
expect((generateCombinedPost as any).recomputations()).toBe(1);
generateCombinedPost(initialState, initialCombinedId);
expect((generateCombinedPost as any).recomputations()).toBe(1);
});
test('should recalculate when called twice with different IDs', () => {
const generateCombinedPost = makeGenerateCombinedPost();
expect((generateCombinedPost as any).recomputations()).toBe(0);
let combinedId = initialCombinedId;
generateCombinedPost(initialState, combinedId);
expect((generateCombinedPost as any).recomputations()).toBe(1);
combinedId = 'user-activity-post2';
generateCombinedPost(initialState, combinedId);
expect((generateCombinedPost as any).recomputations()).toBe(2);
});
test('should not recalculate when a different post changes', () => {
const generateCombinedPost = makeGenerateCombinedPost();
expect((generateCombinedPost as any).recomputations()).toBe(0);
let state = initialState;
generateCombinedPost(state, initialCombinedId);
expect((generateCombinedPost as any).recomputations()).toBe(1);
state = {
...state,
entities: {
...state.entities,
posts: {
...state.entities.posts,
posts: {
...state.entities.posts.posts,
post3: TestHelper.getPostMock({id: 'post3'}),
},
},
},
};
generateCombinedPost(state, initialCombinedId);
expect((generateCombinedPost as any).recomputations()).toBe(2);
});
test('should recalculate when one of the included posts change', () => {
const generateCombinedPost = makeGenerateCombinedPost();
expect((generateCombinedPost as any).recomputations()).toBe(0);
let state = initialState;
generateCombinedPost(state, initialCombinedId);
expect((generateCombinedPost as any).recomputations()).toBe(1);
state = {
...state,
entities: {
...state.entities,
posts: {
...state.entities.posts,
posts: {
...state.entities.posts.posts,
post2: TestHelper.getPostMock({id: 'post2', update_at: 1234}),
},
},
},
};
generateCombinedPost(state, initialCombinedId);
expect((generateCombinedPost as any).recomputations()).toBe(2);
});
});
});
const PostTypes = Posts.POST_TYPES;
describe('extractUserActivityData', () => {