Skip to content

Commit f9bb138

Browse files
RandomByteRandomByte
committed
refactor(project): Improve ResourceRequestGraph handling
1 parent 56341da commit f9bb138

File tree

5 files changed

+60
-119
lines changed

5 files changed

+60
-119
lines changed

‎packages/project/lib/build/cache/BuildTaskCache.js‎

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,7 @@ export default class BuildTaskCache {
282282
requests.push(new Request("dep-patterns", patterns));
283283
}
284284
}
285+
// Try to find an existing request set that we can reuse
285286
let setId = this.#resourceRequests.findExactMatch(requests);
286287
let resourceIndex;
287288
if (setId) {
@@ -301,8 +302,14 @@ export default class BuildTaskCache {
301302
const addedRequests = requestSet.getAddedRequests();
302303
const resourcesToAdd =
303304
await this.#getResourcesForRequests(addedRequests, projectReader, dependencyReader);
305+
if (!resourcesToAdd.length) {
306+
throw new Error(`Unexpected empty added resources for request set ID ${setId} ` +
307+
`of task '${this.#taskName}' of project '${this.#projectName}'`);
308+
}
309+
log.verbose(`Task '${this.#taskName}' of project '${this.#projectName}' ` +
310+
`created derived resource index for request set ID ${setId} ` +
311+
`based on parent ID ${parentId} with ${resourcesToAdd.length} additional resources`);
304312
resourceIndex = await parentResourceIndex.deriveTree(resourcesToAdd);
305-
// await newIndex.add(resourcesToAdd);
306313
} else {
307314
const resourcesRead =
308315
await this.#getResourcesForRequests(requests, projectReader, dependencyReader);
@@ -438,7 +445,7 @@ export default class BuildTaskCache {
438445
* @param {Request[]|Array<{type: string, value: string|string[]}>} resourceRequests - Resource requests to process
439446
* @param {module:@ui5/fs.AbstractReader} projectReader - Reader for project resources
440447
* @param {module:@ui5/fs.AbstractReader} dependencyReder - Reader for dependency resources
441-
* @returns {Promise<IterableIterator<module:@ui5/fs.Resource>>} Iterator of retrieved resources
448+
* @returns {Promise<Array<module:@ui5/fs.Resource>>} Iterator of retrieved resources
442449
* @throws {Error} If an unknown request type is encountered
443450
*/
444451
async #getResourcesForRequests(resourceRequests, projectReader, dependencyReder) {
@@ -477,7 +484,7 @@ export default class BuildTaskCache {
477484
throw new Error(`Unknown request type: ${type}`);
478485
}
479486
}
480-
return resourcesMap.values();
487+
return Array.from(resourcesMap.values());
481488
}
482489

483490
/**
@@ -519,6 +526,10 @@ export default class BuildTaskCache {
519526
* @returns {TaskCacheMetadata} Serialized cache metadata containing the request set graph
520527
*/
521528
toCacheObject() {
529+
if (!this.#resourceRequests) {
530+
throw new Error("BuildTaskCache#toCacheObject: Resource requests not initialized for task " +
531+
`'${this.#taskName}' of project '${this.#projectName}'`);
532+
}
522533
const rootIndices = [];
523534
const deltaIndices = [];
524535
for (const {nodeId, parentId} of this.#resourceRequests.traverseByDepth()) {
@@ -536,6 +547,8 @@ export default class BuildTaskCache {
536547
if (!rootResourceIndex) {
537548
throw new Error(`Missing root resource index for parent ID ${parentId}`);
538549
}
550+
// Store the metadata for all added resources. Note: Those resources might not be available
551+
// in the current tree. In that case we store an empty array.
539552
const addedResourceIndex = resourceIndex.getAddedResourceIndex(rootResourceIndex);
540553
deltaIndices.push({
541554
nodeId,
@@ -567,7 +580,8 @@ export default class BuildTaskCache {
567580
const {resourceIndex: parentResourceIndex} = resourceRequests.getMetadata(node.getParentId());
568581
const registry = registries.get(node.getParentId());
569582
if (!registry) {
570-
throw new Error(`Missing tree registry for parent of node ID ${nodeId}`);
583+
throw new Error(`Missing tree registry for parent of node ID ${nodeId} of task ` +
584+
`'${this.#taskName}' of project '${this.#projectName}'`);
571585
}
572586
const resourceIndex = parentResourceIndex.deriveTreeWithIndex(addedResourceIndex);
573587

‎packages/project/lib/build/cache/ProjectBuildCache.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,7 @@ export default class ProjectBuildCache {
235235
const stageMetadata = await this.#cacheManager.readStageCache(
236236
this.#project.getId(), this.#buildSignature, stageName, stageSignature);
237237
if (stageMetadata) {
238+
log.verbose(`Found cached stage with signature ${stageSignature}`);
238239
const reader = await this.#createReaderForStageCache(
239240
stageName, stageSignature, stageMetadata.resourceMetadata);
240241
return {

‎packages/project/lib/build/cache/ResourceRequestGraph.js‎

Lines changed: 24 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -187,55 +187,6 @@ export default class ResourceRequestGraph {
187187
return Array.from(this.nodes.keys());
188188
}
189189

190-
/**
191-
* Find the best parent for a new request set using greedy selection
192-
*
193-
* @param {Request[]} requestSet - Array of Request objects
194-
* @returns {{parentId: number, deltaSize: number}|null} Parent info or null if no suitable parent
195-
*/
196-
findBestParent(requestSet) {
197-
if (this.nodes.size === 0) {
198-
return null;
199-
}
200-
201-
const requestKeys = new Set(requestSet.map((r) => r.toKey()));
202-
let bestParent = null;
203-
let smallestDelta = Infinity;
204-
205-
// Compare against all existing nodes
206-
for (const [nodeId, node] of this.nodes) {
207-
const nodeSet = node.getMaterializedSet(this);
208-
209-
// Calculate how many new requests would need to be added
210-
const delta = this._calculateDelta(requestKeys, nodeSet);
211-
212-
// We want the parent that minimizes the delta (maximum overlap)
213-
if (delta < smallestDelta) {
214-
smallestDelta = delta;
215-
bestParent = nodeId;
216-
}
217-
}
218-
219-
return bestParent !== null ? {parentId: bestParent, deltaSize: smallestDelta} : null;
220-
}
221-
222-
/**
223-
* Calculate the size of the delta (requests in newSet not in existingSet)
224-
*
225-
* @param {Set<string>} newSetKeys - Set of request keys
226-
* @param {Set<string>} existingSetKeys - Set of existing request keys
227-
* @returns {number} Number of requests in newSet not in existingSet
228-
*/
229-
_calculateDelta(newSetKeys, existingSetKeys) {
230-
let deltaCount = 0;
231-
for (const key of newSetKeys) {
232-
if (!existingSetKeys.has(key)) {
233-
deltaCount++;
234-
}
235-
}
236-
return deltaCount;
237-
}
238-
239190
/**
240191
* Calculate which requests need to be added (delta)
241192
*
@@ -245,15 +196,9 @@ export default class ResourceRequestGraph {
245196
*/
246197
_calculateAddedRequests(newRequestSet, parentSet) {
247198
const newKeys = new Set(newRequestSet.map((r) => r.toKey()));
248-
const addedKeys = [];
199+
const addedKeys = newKeys.difference(parentSet);
249200

250-
for (const key of newKeys) {
251-
if (!parentSet.has(key)) {
252-
addedKeys.push(key);
253-
}
254-
}
255-
256-
return addedKeys.map((key) => Request.fromKey(key));
201+
return Array.from(addedKeys).map((key) => Request.fromKey(key));
257202
}
258203

259204
/**
@@ -267,53 +212,56 @@ export default class ResourceRequestGraph {
267212
const nodeId = this.nextId++;
268213

269214
// Find best parent
270-
const parentInfo = this.findBestParent(requests);
215+
const parentId = this.findBestParent(requests);
271216

272-
if (parentInfo === null) {
217+
if (parentId === null) {
273218
// No existing nodes, or no suitable parent - create root node
274219
const node = new RequestSetNode(nodeId, null, requests, metadata);
275220
this.nodes.set(nodeId, node);
276221
return nodeId;
277222
}
278223

279224
// Create node with delta from best parent
280-
const parentNode = this.getNode(parentInfo.parentId);
225+
const parentNode = this.getNode(parentId);
281226
const parentSet = parentNode.getMaterializedSet(this);
282227
const addedRequests = this._calculateAddedRequests(requests, parentSet);
283228

284-
const node = new RequestSetNode(nodeId, parentInfo.parentId, addedRequests, metadata);
229+
const node = new RequestSetNode(nodeId, parentId, addedRequests, metadata);
285230
this.nodes.set(nodeId, node);
286231

287232
return nodeId;
288233
}
289234

290235
/**
291-
* Find the best matching node for a query request set
292-
* Returns the node ID where the node's set is a subset of the query
293-
* and is maximal (largest subset match)
236+
* Find the best parent for a new request set. That is, the largest subset of the new request set.
294237
*
295-
* @param {Request[]} queryRequests - Array of Request objects to match
296-
* @returns {number|null} Node ID of best match, or null if no match found
238+
* @param {Request[]} requestSet - Array of Request objects
239+
* @returns {{parentId: number, deltaSize: number}|null} Parent info or null if no suitable parent
297240
*/
298-
findBestMatch(queryRequests) {
299-
const queryKeys = new Set(queryRequests.map((r) => r.toKey()));
241+
findBestParent(requestSet) {
242+
if (this.nodes.size === 0) {
243+
return null;
244+
}
300245

301-
let bestMatch = null;
302-
let bestMatchSize = -1;
246+
const queryKeys = new Set(requestSet.map((r) => r.toKey()));
247+
let bestParent = null;
248+
let greatestSubset = -1;
303249

250+
// Compare against all existing nodes
304251
for (const [nodeId, node] of this.nodes) {
305252
const nodeSet = node.getMaterializedSet(this);
306253

307254
// Check if nodeSet is a subset of queryKeys
308-
const isSubset = this._isSubset(nodeSet, queryKeys);
255+
const isSubset = nodeSet.isSubsetOf(queryKeys);
309256

310-
if (isSubset && nodeSet.size > bestMatchSize) {
311-
bestMatch = nodeId;
312-
bestMatchSize = nodeSet.size;
257+
// We want the parent the greatest overlap
258+
if (isSubset && nodeSet.size > greatestSubset) {
259+
bestParent = nodeId;
260+
greatestSubset = nodeSet.size;
313261
}
314262
}
315263

316-
return bestMatch;
264+
return bestParent;
317265
}
318266

319267
/**
@@ -338,30 +286,14 @@ export default class ResourceRequestGraph {
338286
}
339287

340288
// Check if sets are identical (same size + subset = equality)
341-
if (this._isSubset(nodeSet, queryKeys)) {
289+
if (nodeSet.isSubsetOf(queryKeys)) {
342290
return nodeId;
343291
}
344292
}
345293

346294
return null;
347295
}
348296

349-
/**
350-
* Check if setA is a subset of setB
351-
*
352-
* @param {Set<string>} setA - First set
353-
* @param {Set<string>} setB - Second set
354-
* @returns {boolean} True if setA is a subset of setB
355-
*/
356-
_isSubset(setA, setB) {
357-
for (const item of setA) {
358-
if (!setB.has(item)) {
359-
return false;
360-
}
361-
}
362-
return true;
363-
}
364-
365297
/**
366298
* Get metadata associated with a node
367299
*

‎packages/project/test/lib/build/cache/ResourceRequestGraph.js‎

Lines changed: 17 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -155,18 +155,17 @@ test("ResourceRequestGraph: Add request set with parent relationship", (t) => {
155155
t.true(node2Data.addedRequests.has("path:c.js"));
156156
});
157157

158-
test("ResourceRequestGraph: Add request set with no overlap creates parent", (t) => {
158+
test("ResourceRequestGraph: Add request set with no overlap", (t) => {
159159
const graph = new ResourceRequestGraph();
160160

161161
const set1 = [new Request("path", "a.js")];
162-
const node1 = graph.addRequestSet(set1);
162+
graph.addRequestSet(set1);
163163

164164
const set2 = [new Request("path", "x.js")];
165165
const node2 = graph.addRequestSet(set2);
166166

167167
const node2Data = graph.getNode(node2);
168-
// Even with no overlap, greedy algorithm will select best parent
169-
t.is(node2Data.parent, node1);
168+
t.falsy(node2Data.parent);
170169
t.is(node2Data.addedRequests.size, 1);
171170
t.true(node2Data.addedRequests.has("path:x.js"));
172171
});
@@ -218,7 +217,7 @@ test("ResourceRequestGraph: getAddedRequests returns only delta", (t) => {
218217
t.is(added[0].toKey(), "path:c.js");
219218
});
220219

221-
test("ResourceRequestGraph: findBestMatch returns node with largest subset", (t) => {
220+
test("ResourceRequestGraph: findBestParent returns node with largest subset", (t) => {
222221
const graph = new ResourceRequestGraph();
223222

224223
// Add first request set
@@ -243,13 +242,13 @@ test("ResourceRequestGraph: findBestMatch returns node with largest subset", (t)
243242
new Request("path", "c.js"),
244243
new Request("path", "x.js")
245244
];
246-
const match = graph.findBestMatch(query);
245+
const match = graph.findBestParent(query);
247246

248247
// Should return node2 (largest subset: 3 items)
249248
t.is(match, node2);
250249
});
251250

252-
test("ResourceRequestGraph: findBestMatch returns null when no subset found", (t) => {
251+
test("ResourceRequestGraph: findBestParent returns null when no subset found", (t) => {
253252
const graph = new ResourceRequestGraph();
254253

255254
const set1 = [
@@ -263,7 +262,7 @@ test("ResourceRequestGraph: findBestMatch returns null when no subset found", (t
263262
new Request("path", "x.js"),
264263
new Request("path", "y.js")
265264
];
266-
const match = graph.findBestMatch(query);
265+
const match = graph.findBestParent(query);
267266

268267
t.is(match, null);
269268
});
@@ -416,7 +415,7 @@ test("ResourceRequestGraph: getStats returns correct statistics", (t) => {
416415
t.is(stats.compressionRatio, 0.6); // 3 stored / 5 total
417416
});
418417

419-
test("ResourceRequestGraph: toMetadataObject exports graph structure", (t) => {
418+
test("ResourceRequestGraph: toCacheObject exports graph structure", (t) => {
420419
const graph = new ResourceRequestGraph();
421420

422421
const set1 = [new Request("path", "a.js")];
@@ -428,7 +427,7 @@ test("ResourceRequestGraph: toMetadataObject exports graph structure", (t) => {
428427
];
429428
const node2 = graph.addRequestSet(set2);
430429

431-
const exported = graph.toMetadataObject();
430+
const exported = graph.toCacheObject();
432431

433432
t.is(exported.nodes.length, 2);
434433
t.is(exported.nextId, 3);
@@ -444,7 +443,7 @@ test("ResourceRequestGraph: toMetadataObject exports graph structure", (t) => {
444443
t.deepEqual(exportedNode2.addedRequests, ["path:b.js"]);
445444
});
446445

447-
test("ResourceRequestGraph: fromMetadataObject reconstructs graph", (t) => {
446+
test("ResourceRequestGraph: fromCacheObject reconstructs graph", (t) => {
448447
const graph1 = new ResourceRequestGraph();
449448

450449
const set1 = [new Request("path", "a.js")];
@@ -457,8 +456,8 @@ test("ResourceRequestGraph: fromMetadataObject reconstructs graph", (t) => {
457456
const node2 = graph1.addRequestSet(set2);
458457

459458
// Export and reconstruct
460-
const exported = graph1.toMetadataObject();
461-
const graph2 = ResourceRequestGraph.fromMetadataObject(exported);
459+
const exported = graph1.toCacheObject();
460+
const graph2 = ResourceRequestGraph.fromCacheObject(exported);
462461

463462
// Verify reconstruction
464463
t.is(graph2.nodes.size, 2);
@@ -542,15 +541,15 @@ test("ResourceRequestGraph: findBestParent chooses optimal parent", (t) => {
542541

543542
// Create two potential parents
544543
const set1 = [
545-
new Request("path", "a.js"),
546-
new Request("path", "b.js")
544+
new Request("path", "x.js"),
545+
new Request("path", "y.js")
547546
];
548547
graph.addRequestSet(set1);
549548

550549
const set2 = [
551550
new Request("path", "x.js"),
552551
new Request("path", "y.js"),
553-
new Request("path", "z.js")
552+
new Request("path", "z.js"),
554553
];
555554
const node2 = graph.addRequestSet(set2);
556555

@@ -607,7 +606,7 @@ test("ResourceRequestGraph: Caching works correctly", (t) => {
607606
t.deepEqual(Array.from(materialized1).sort(), Array.from(materialized2).sort());
608607
});
609608

610-
test("ResourceRequestGraph: Usage example from documentation", (t) => {
609+
test("ResourceRequestGraph: Integration", (t) => {
611610
// Create graph
612611
const graph = new ResourceRequestGraph();
613612

@@ -637,9 +636,8 @@ test("ResourceRequestGraph: Usage example from documentation", (t) => {
637636
const query = [
638637
new Request("path", "a.js"),
639638
new Request("path", "b.js"),
640-
new Request("path", "x.js")
641639
];
642-
const match = graph.findBestMatch(query);
640+
const match = graph.findExactMatch(query);
643641
t.is(match, node1);
644642

645643
// Get metadata

0 commit comments

Comments
 (0)