Skip to content

Commit

Permalink
Merge pull request #411 from AllenInstitute/bugfix/cache-modal-perfor…
Browse files Browse the repository at this point in the history 
…mance

bugfix/cache-modal-performance
  • Loading branch information
@BrianWhitneyAI
BrianWhitneyAI authored Feb 6, 2025
2 parents d605b1e + 9a50a17 commit e236819
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 47 deletions.
70 changes: 49 additions & 21 deletions packages/core/components/Modal/CopyFileManifest/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ function FileTable({ files, title }: { files: FileDetail[]; title: string }) {
};

const calculateTotalSize = (files: FileDetail[]) => {
if (files.length === 0) return "";
const totalBytes = files.reduce((acc, file) => acc + (file.size || 0), 0);
return totalBytes ? filesize(totalBytes) : "Calculating...";
};
Expand Down Expand Up @@ -72,9 +71,7 @@ function FileTable({ files, title }: { files: FileDetail[]; title: string }) {
{hasScroll && <div className={styles.gradientOverlay} />}
</div>
<div className={styles.summary}>
{files.length > 0 && (
<span className={styles.totalSize}>{calculateTotalSize(files)}</span>
)}
<span className={styles.totalSize}>{calculateTotalSize(files)}</span>
<span className={styles.fileCount}>{files.length.toLocaleString()} files</span>
</div>
</div>
Expand All @@ -92,13 +89,22 @@ export default function CopyFileManifest({ onDismiss }: ModalProps) {
);

const [fileDetails, setFileDetails] = React.useState<FileDetail[]>([]);
const [loading, setLoading] = React.useState<boolean>(true);
const [error, setError] = React.useState<string | null>(null);

React.useEffect(() => {
async function fetchDetails() {
const details = await fileSelection.fetchAllDetails();
setFileDetails(details);
async function fetchFiles() {
try {
const details = await fileSelection.fetchAllDetails();
setFileDetails(details);
} catch (err: any) {
console.error("Error fetching file details:", err);
setError(err.message || "Error fetching file details.");
} finally {
setLoading(false);
}
}
fetchDetails();
fetchFiles();
}, [fileSelection]);

const onMove = () => {
Expand All @@ -124,18 +130,42 @@ export default function CopyFileManifest({ onDismiss }: ModalProps) {

return (
<BaseModal
title="Copy files to local storage (VAST)"
onDismiss={onDismiss}
body={
<div className={styles.bodyContainer}>
<p className={styles.note}>
Files copied to the local storage (VAST) are stored with a 180-day
expiration, after which they revert to cloud-only storage. To extend the
expiration, reselect the files and confirm the update.
</p>
<FileTable
files={filesInLocalCache}
title="Files that are already on VAST: Extend expiration"
/>
<FileTable files={filesNotInLocalCache} title="Files to download to VAST" />
{loading && (
<div className={styles.loading}>
<p>Loading file details...</p>
</div>
)}
{error && (
<div className={styles.errorContainer}>
<h3>Error</h3>
<p>{error}</p>
</div>
)}
{!loading && !error && (
<>
<p className={styles.note}>
Files copied to the local storage (VAST) are stored with a 180-day
expiration, after which they revert to cloud-only storage. To extend
the expiration, reselect the files and confirm the update.
</p>
{filesInLocalCache.length > 0 && (
<FileTable
files={filesInLocalCache}
title="Files that are already on VAST: Extend expiration"
/>
)}
{filesNotInLocalCache.length > 0 && (
<FileTable
files={filesNotInLocalCache}
title="Files to download to VAST"
/>
)}
</>
)}
</div>
}
footer={
Expand All @@ -148,15 +178,13 @@ export default function CopyFileManifest({ onDismiss }: ModalProps) {
/>
<PrimaryButton
className={styles.confirmButton}
disabled={!fileDetails.length}
disabled={loading || !!error}
onClick={onMove}
text="CONFIRM"
title=""
/>
</div>
}
onDismiss={onDismiss}
title="Copy files to local storage (VAST)"
/>
);
}
18 changes: 2 additions & 16 deletions packages/core/entity/FileSelection/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -210,27 +210,13 @@ export default class FileSelection {
* Fetch metadata for all items selected.
*/
public async fetchAllDetails(): Promise<FileDetail[]> {
const fileRangesByFileSets = this.groupByFileSet();
// Load file metadata for every file selected (however do to some performance enhancements
// the fetch will overshoot)
const fileRangePromises: Promise<FileDetail[]>[] = [];
fileRangesByFileSets.forEach((ranges, fileSet) => {
this.groupByFileSet().forEach((ranges, fileSet) => {
ranges.forEach((range) => {
fileRangePromises.push(fileSet.fetchFileRange(range.from, range.to));
});
});
await Promise.all(fileRangePromises);

// Collect the desired files from the fetched files
const files: FileDetail[] = [];
fileRangesByFileSets.forEach((ranges, fileSet) => {
ranges.forEach((range) => {
for (let i = range.from; i <= range.to; i++) {
files.push(fileSet.getFileByIndex(i) as FileDetail);
}
});
});
return files;
return (await Promise.all(fileRangePromises)).flat();
}

/**
Expand Down
27 changes: 19 additions & 8 deletions packages/core/entity/FileSet/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,14 +121,25 @@ export default class FileSet {
fileSet: this,
});

// Update cache for files fetched, due to overfetching the indexes updated
// in the cache will be inclusive of the range requested but may not necessarily start
// at the requested index, therefore here startIndexOfPage is used instead of startIndex
for (let i = 0; i < response.length; i++) {
this.cache.set(startIndexOfPage + i, response[i]);
}

return response;
// Grab responses and shove them into the cache
// while also filtering out the files that outside the range
// requested for the response to the caller of fetchFileRange()
return response.reduce((filesInRange, file, idxInResponses) => {
// Update cache for files fetched, due to overfetching the indexes updated
// in the cache will be inclusive of the range requested but may not necessarily start
// at the requested index, therefore here startIndexOfPage is used instead of startIndex
const idxInFileSet = startIndexOfPage + idxInResponses;
this.cache.set(idxInFileSet, file);

// If the file is within the range requested, add it to the files array
// for returning back to the caller of fetchFileRange().
// Ranges are inclusive of their bounds
if (idxInFileSet >= startIndex && idxInFileSet <= endIndex) {
filesInRange.push(file);
}

return filesInRange;
}, [] as FileDetail[]);
} finally {
// Clear the previously saved indexes as they are no longer loading
for (let i = startIndexOfPage; i < startIndexOfPage + limit; i++) {
Expand Down
4 changes: 2 additions & 2 deletions packages/core/entity/FileSet/test/FileSet.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,9 +143,9 @@ describe("FileSet", () => {

it("returns slices of the file list represented by the FileSet, specified by index position", async () => {
const fileService = new HttpFileService();
sandbox.replace(fileService, "getFiles", () => Promise.resolve(files.slice(1, 4)));
sandbox.replace(fileService, "getFiles", () => Promise.resolve(files.slice(0, 4)));
const fileSet = new FileSet({ fileService });
expect(await fileSet.fetchFileRange(1, 3)).to.deep.equal(files.slice(1, 4)); // Array.prototype.slice is exclusive of end bound
expect(await fileSet.fetchFileRange(1, 3)).to.deep.equal(files.slice(1, 4));
});

it("turns indicies for requested data into a properly formed pagination query", async () => {
Expand Down

0 comments on commit e236819

Please sign in to comment.