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

Feat/discovery load time spike #1579

Draft
wants to merge 18 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
55f62d2
feat(discoveryLoadTimeSpike): began creating prototype
jarvisraymond-uchicago Aug 1, 2024
1fd18a4
feat(discoveryLoadTimeSpike): added progress bar
jarvisraymond-uchicago Aug 2, 2024
5332d9a
feat(discoveryLoadTimeSpike): Added progress bars that can be toggle …
jarvisraymond-uchicago Aug 8, 2024
8d70ce9
feat(discoveryLoadTimeSpike): Added logic to only batch load if condi…
jarvisraymond-uchicago Aug 9, 2024
f51fc1c
feat(discoveryLoadTimeSpike): Removed autoformatting from DiscoverySu…
jarvisraymond-uchicago Aug 9, 2024
a64f848
feat(discoveryLoadTimeSpike): Removed autoformatting from index.tsx
jarvisraymond-uchicago Aug 9, 2024
5be3df0
feat(discoveryLoadTimeSpike): Removed autoformatting from Discovery.tsx
jarvisraymond-uchicago Aug 9, 2024
ca1f953
feat(discoveryLoadTimeSpike): Added injected CSS to make it look nice…
jarvisraymond-uchicago Aug 9, 2024
187d30f
feat(discoveryLoadTimeSpike): Updated progress bar colors and progres…
jarvisraymond-uchicago Aug 16, 2024
8684e1a
feat(discoveryLoadTimeSpike): Adjusted processingTimeDelay
jarvisraymond-uchicago Aug 16, 2024
0629ce3
feat(discoveryLoadTimeSpike): updated logic to hide progress bar to u…
jarvisraymond-uchicago Aug 16, 2024
25734db
feat(discoveryLoadTimeSpike): updated variables to try to make progre…
jarvisraymond-uchicago Aug 16, 2024
fa90e3e
feat(discoveryLoadTimeSpike): Moved comment to relevant code
jarvisraymond-uchicago Aug 22, 2024
799e9c2
feat(discoveryLoadTimeSpike): Removed DiscoveryLoadingProgress, only …
jarvisraymond-uchicago Aug 23, 2024
905b7e1
feat(discoveryLoadTimeSpike): added explanatory comments, cleaned up …
jarvisraymond-uchicago Aug 23, 2024
0b7a4d7
feat(discoveryLoadTimeSpike): Reverted footer file
jarvisraymond-uchicago Aug 29, 2024
06d7acc
feat(discoveryLoadTimeSpike): updated so the change is globally enabl…
jarvisraymond-uchicago Aug 30, 2024
fd8a086
feat(discoveryLoadTimeSpike): Updated name of studiesBatchCount to nu…
jarvisraymond-uchicago Sep 12, 2024
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
54 changes: 36 additions & 18 deletions src/Discovery/Discovery.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import React, {
import * as JsSearch from 'js-search';
import jsonpath from 'jsonpath';
import {
Tag, Popover, Space, Collapse, Button, Dropdown, Pagination, Tooltip,
Tag, Popover, Space, Collapse, Button, Dropdown, Pagination, Tooltip, Spin,
} from 'antd';
import {
LockOutlined,
Expand Down Expand Up @@ -215,6 +215,7 @@ export interface Props {
onAccessSortDirectionSet: (accessSortDirection: AccessSortDirection) => any,
onResourcesSelected: (resources: DiscoveryResource[]) => any,
onPaginationSet: (pagination: { currentPage: number, resultsPerPage: number }) => any,
batchLoadingInfo: {allBatchesAreLoaded: boolean},
}

const Discovery: React.FunctionComponent<Props> = (props: Props) => {
Expand All @@ -237,6 +238,13 @@ const Discovery: React.FunctionComponent<Props> = (props: Props) => {
);
const [visibleResources, setVisibleResources] = useState([]);

const batchesAreLoading = props.batchLoadingInfo.allBatchesAreLoaded === false;
const BatchLoadingSpinner = () => (
<div style={{ textAlign: 'center' }}>
<Spin />
</div>
);

const handleSearchChange = (ev) => {
const { value } = ev.currentTarget;
props.onSearchChange(value);
Expand Down Expand Up @@ -656,6 +664,7 @@ const Discovery: React.FunctionComponent<Props> = (props: Props) => {
{/* Header with stats */}
<div className='discovery-header'>
<DiscoverySummary
batchLoadingInfo={props.batchLoadingInfo}
visibleResources={visibleResources}
config={config}
/>
Expand Down Expand Up @@ -696,12 +705,18 @@ const Discovery: React.FunctionComponent<Props> = (props: Props) => {
<Collapse activeKey={(searchableTagCollapsed) ? '' : '1'} ghost>
<Panel className='discovery-header__dropdown-tags-display-panel' header='' key='1'>
<div className='discovery-header__dropdown-tags'>
<DiscoveryDropdownTagViewer
config={config}
studies={props.studies}
selectedTags={props.selectedTags}
setSelectedTags={props.onTagsSelected}
/>
{ batchesAreLoading
? (
<BatchLoadingSpinner />
)
: (
<DiscoveryDropdownTagViewer
config={config}
studies={props.studies}
selectedTags={props.selectedTags}
setSelectedTags={props.onTagsSelected}
/>
)}
</div>
</Panel>
</Collapse>
Expand Down Expand Up @@ -751,17 +766,20 @@ const Discovery: React.FunctionComponent<Props> = (props: Props) => {
<div
className='discovery-filters--visible'
>
<DiscoveryAdvancedSearchPanel
config={props.config}
studies={props.studies}
filterState={filterState}
setFilterState={(event) => {
props.onAdvancedSearch(event);
setFilterState(event);
}}
filterMultiSelectionLogic={filterMultiSelectionLogic}
setFilterMultiSelectionLogic={setFilterMultiSelectionLogic}
/>
{batchesAreLoading ? <BatchLoadingSpinner />
: (
<DiscoveryAdvancedSearchPanel
config={props.config}
studies={props.studies}
filterState={filterState}
setFilterState={(event) => {
props.onAdvancedSearch(event);
setFilterState(event);
}}
filterMultiSelectionLogic={filterMultiSelectionLogic}
setFilterMultiSelectionLogic={setFilterMultiSelectionLogic}
/>
)}
</div>
) : (<div className='discovery-filters--hide' />)}

Expand Down
69 changes: 69 additions & 0 deletions src/Discovery/DiscoveryLoadingProgressMini.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
import React, { useState, useEffect } from 'react';
import { Progress } from 'antd';

interface DiscoveryLoadingProps {
batchLoadingInfo: { allBatchesAreLoaded: boolean }
}

// this should probably be done in a CSS for production:
const style = document.createElement('style');
style.type = 'text/css';
const css = '.discovery-header__dropdown-tags-container {margin-top: 15px;} .discovery-header{align-items:start;} ';
style.innerHTML = css;
document.head.appendChild(style);

const DiscoveryLoadingProgressMini = ({
batchLoadingInfo,
}: DiscoveryLoadingProps) => {
const [percent, setPercent] = useState(0);
const { allBatchesAreLoaded } = batchLoadingInfo;
const [displayProgressBar, setDisplayProgressBar] = useState(true);

// Fake loading UI
const percentUpdateInterval = 500;
const percentIncrementAmount = 5;
useEffect(() => {
const interval = setInterval(() => {
setPercent((prevPercent) => prevPercent + percentIncrementAmount);
}, percentUpdateInterval);
return () => clearInterval(interval);
}, [percent, allBatchesAreLoaded]);

// hide the bar with a transition delay after the batches are loaded,
// giving the browser some time to process the batch
const delayTimeBeforeHidingProgressBar = 2000;
useEffect(() => {
if (allBatchesAreLoaded) {
// Change displayProgressBar to false after delay
setTimeout(() => {
setDisplayProgressBar(false);
}, delayTimeBeforeHidingProgressBar);
}
}, [allBatchesAreLoaded]);

const progressContainerStyle = {
textAlign: 'center',
marginBottom: '5px',
display: displayProgressBar ? 'block' : 'none',
};

return (
<div style={progressContainerStyle}>
<Progress
width={80}
showInfo={false}
percent={allBatchesAreLoaded ? 100 : percent}
status='success'
strokeColor='#99286B'
/>
<p
style={{ lineHeight: 'normal', textTransform: 'inherit' }}
className='discovery-header__stat-label'
>
Loading studies...
</p>
</div>
);
};

export default DiscoveryLoadingProgressMini;
6 changes: 6 additions & 0 deletions src/Discovery/DiscoverySummary.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import uniq from 'lodash/uniq';
import sum from 'lodash/sum';
import jsonpath from 'jsonpath';
import { DiscoveryConfig } from './DiscoveryConfig';
import DiscoveryLoadingProgressMini from './DiscoveryLoadingProgressMini';

/**
* Check for non-numeric items in an array and convert them to numbers.
Expand Down Expand Up @@ -55,6 +56,7 @@ const renderAggregation = (aggregation: AggregationConfig, studies: any[] | null
interface Props {
visibleResources: any[] | null;
config: DiscoveryConfig;
batchLoadingInfo: { allBatchesAreLoaded: boolean },
}

const DiscoverySummary = (props: Props) => (
Expand All @@ -72,6 +74,10 @@ const DiscoverySummary = (props: Props) => (
{aggregation.name}
</div>
</div>
<DiscoveryLoadingProgressMini
batchLoadingInfo={props.batchLoadingInfo}
/>

</div>
</React.Fragment>
))
Expand Down
28 changes: 26 additions & 2 deletions src/Discovery/MDSUtils.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { mdsURL, studyRegistrationConfig } from '../localconf';
const LIMIT = 2000; // required or else mds defaults to returning 10 records
const STUDY_DATA_FIELD = 'gen3_discovery'; // field in the MDS response that contains the study data

const loadStudiesFromMDS = async (guidType = 'discovery_metadata') => {
export const loadStudiesFromMDS = async (guidType = 'discovery_metadata') => {
try {
let allStudies = [];
let offset = 0;
Expand Down Expand Up @@ -43,4 +43,28 @@ const loadStudiesFromMDS = async (guidType = 'discovery_metadata') => {
}
};

export default loadStudiesFromMDS;
export const getSomeStudiesFromMDS = async (guidType = 'discovery_metadata', numberOfStudies = 10) => {
try {
let allStudies = [];
// request up to LIMIT studies from MDS at a time.
const url = `${mdsURL}?data=True&_guid_type=${guidType}&limit=${numberOfStudies}`;
const res = await fetch(url);
if (res.status !== 200) {
throw new Error(`Request for study data at ${url} failed. Response: ${JSON.stringify(res, null, 2)}`);
}
// eslint-disable-next-line no-await-in-loop
const jsonResponse = await res.json();
const studies = Object.values(jsonResponse).map((entry) => {
const study = { ...entry[STUDY_DATA_FIELD] };
// copy VLMD info if exists
if (studyRegistrationConfig?.dataDictionaryField && entry[studyRegistrationConfig.dataDictionaryField]) {
study[studyRegistrationConfig.dataDictionaryField] = entry[studyRegistrationConfig.dataDictionaryField];
}
return study;
});
allStudies = allStudies.concat(studies);
return allStudies;
} catch (err) {
throw new Error(`Request for study data failed: ${err}`);
}
};
14 changes: 9 additions & 5 deletions src/Discovery/aggMDSUtils.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const getUniqueTags = ((tags) => tags.filter((v, i, a) => a.findIndex((t) => (
t.name?.length > 0 && t.category === v.category && t.name === v.name)) === i));

const loadStudiesFromAggMDSRequests = async (offset, limit) => {
var startTimeloadStudiesFromAggMDSRequests= performance.now();
const url = `${aggMDSDataURL}?data=True&limit=${limit}&offset=${offset}`;

const res = await fetch(url);
Expand Down Expand Up @@ -64,18 +65,21 @@ const loadStudiesFromAggMDSRequests = async (offset, limit) => {

allStudies = allStudies.concat(editedStudies);
});

var endTimeloadStudiesFromAggMDSRequests = performance.now();
console.log(
`Call to loadStudiesFromAggMDSRequests took ${endTimeloadStudiesFromAggMDSRequests - startTimeloadStudiesFromAggMDSRequests} milliseconds`
);
return allStudies;
};

const loadStudiesFromAggMDS = async () => {
const loadStudiesFromAggMDS = async (limit=2000) => {
// Retrieve from aggregate MDS

const offset = 0; // For pagination
const limit = 2000; // Total number of rows requested

// const limit = 2000; // Total number of rows requested
// const limit = 5;
var startTimeloadStudiesFromAggMDSRequests= performance.now();
const studies = await loadStudiesFromAggMDSRequests(offset, limit);

return studies;
};

Expand Down
66 changes: 56 additions & 10 deletions src/Discovery/index.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import React, { useState, useEffect } from 'react';
import { connect } from 'react-redux';
import _ from 'lodash';

import Discovery, { AccessLevel, AccessSortDirection, DiscoveryResource } from './Discovery';
import { DiscoveryConfig } from './DiscoveryConfig';
import { userHasMethodForServiceOnResource } from '../authMappingUtils';
Expand All @@ -10,7 +9,7 @@ import {
} from '../localconf';
import isEnabled from '../helpers/featureFlags';
import loadStudiesFromAggMDS from './aggMDSUtils';
import loadStudiesFromMDS from './MDSUtils';
import { loadStudiesFromMDS, getSomeStudiesFromMDS } from './MDSUtils';

const populateStudiesWithConfigInfo = (studies, config) => {
if (!config.studies) {
Expand Down Expand Up @@ -73,22 +72,58 @@ const DiscoveryWithMDSBackend: React.FC<{
throw new Error('Could not find configuration for Discovery page. Check the portal config.');
}

// Downloads and processes studies in two seperate batches
// to improve load time & usability
// Initially uses a smaller batch to load interface quickly
// Then a batch with all the studies
const [studiesBatchCount, setStudiesBatchCount] = useState(0);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: Update the variable name to "numberOfBatchesLoaded"

const expectedNumberOfTotalBatches = 2;
const numberOfStudiesForSmallerBatch = 5;
const numberOfStudiesForAllStudiesBatch = 2000;

useEffect(() => {
// If batch loading is Enabled, update the studiesBatchCount to enable calling of different batch sizes
// with different parameters
if (studiesBatchCount < expectedNumberOfTotalBatches) setStudiesBatchCount(studiesBatchCount + 1);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: Update the variable name to "numberOfBatchesLoaded"


const studyRegistrationValidationField = studyRegistrationConfig?.studyRegistrationValidationField;
async function fetchRawStudies() {
let loadStudiesFunction;
const startTime = performance.now();
let loadStudiesFunction: Function;
let loadStudiesParameters: any;
if (isEnabled('discoveryUseAggMDS')) {
loadStudiesFunction = loadStudiesFromAggMDS;
loadStudiesParameters = studiesBatchCount === 1
? numberOfStudiesForSmallerBatch
: numberOfStudiesForAllStudiesBatch;
} else {
loadStudiesFunction = loadStudiesFromMDS;
loadStudiesFunction = getSomeStudiesFromMDS;
loadStudiesParameters = props.config?.features?.guidType;
}
const rawStudiesRegistered = await loadStudiesFunction(props.config?.features?.guidType);
let rawStudiesUnregistered = [];
const rawStudiesRegistered = await loadStudiesFunction(
loadStudiesParameters,
);
let rawStudiesUnregistered: any[] = [];
if (isEnabled('studyRegistration')) {
rawStudiesUnregistered = await loadStudiesFromMDS('unregistered_discovery_metadata');
rawStudiesUnregistered = rawStudiesUnregistered
.map((unregisteredStudy) => ({ ...unregisteredStudy, [studyRegistrationValidationField]: false }));
// Load fewer raw studies if on the first studies batch
// Otherwise load them all
rawStudiesUnregistered = studiesBatchCount === 1
? (rawStudiesUnregistered = await getSomeStudiesFromMDS(
'unregistered_discovery_metadata',
numberOfStudiesForSmallerBatch,
))
: await loadStudiesFromMDS('unregistered_discovery_metadata');
rawStudiesUnregistered = rawStudiesUnregistered.map(
(unregisteredStudy) => ({
...unregisteredStudy,
[studyRegistrationValidationField]: false,
}),
);
}
const endTime = performance.now();
console.log(
`Call to fetchRawStudies took ${endTime - startTime} milliseconds`,
);
return _.union(rawStudiesRegistered, rawStudiesUnregistered);
}
fetchRawStudies().then((rawStudies) => {
Expand Down Expand Up @@ -167,16 +202,27 @@ const DiscoveryWithMDSBackend: React.FC<{

// indicate discovery tag is active even if we didn't click a button to get here
props.onDiscoveryPageActive();
}, []);
}, [props, studiesBatchCount]);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: Update the variable name to "numberOfBatchesLoaded"


let studyRegistrationValidationField = studyRegistrationConfig?.studyRegistrationValidationField;
if (!isEnabled('studyRegistration')) {
studyRegistrationValidationField = undefined;
}

const batchLoadingInfo = {
// All batches all loaded if the studies are not null and
// their length is great than the studies for the smaller batches
// from loadStudiesFromAggMDS and getSomeStudiesFromMDS
allBatchesAreLoaded: studies === null
? false
: studies?.length > numberOfStudiesForSmallerBatch * 2,
};

return (
<Discovery
studies={studies === null ? [] : studies}
studyRegistrationValidationField={studyRegistrationValidationField}
batchLoadingInfo={batchLoadingInfo}
{...props}
/>
);
Expand Down
Loading