Skip to content

Commit 9feafd8

Browse files
committed
introduce new Publisher for incremental delivery
Depends on #3784 The proposed new Publisher: 1. does not use the event loop to manage AsyncRecord dependencies 2. performs as much work as possible synchronously 2. uses separate sets to store pending vs ready AsyncRecords 3. does not use `Promise.race` -- No event loop for managing AsyncRecord dependencies The current Publisher wraps every AsyncRecord result in a promise that only resolves after the parent AsyncRecord resolves. If multiple items within a stream are released for publishing because their parent has just been published, the stream cannot be in its entirety until after all of these promises unwind. The new Publisher keeps track of dependencies manually. When an AsyncRecord is pushed by the publisher, all of its dependent AsyncRecords are synchronously pushed, repeating as necessary, without using the event loop. In general, the new publisher aims to perform as much work as possible synchronously. -- Separate sets for pending vs ready AsyncRecords The current publisher inspects all pending AsyncRecords whenever any of them resolves. All that are completed are added to the response. The new Publisher moves AsyncRecords from the pending set to the ready set as they are pushed, so that on each call to next, only the ready set must be iterated. As a side-effect of this change, the incremental array is ordered by which items are ready for delivery first, and not by the initial document. This seems like a worthwhile tradeoff, and is still adherent to the spec, as far as I can tell. -- No `Promise.race` The old Publisher uses `Promise.race` as the trigger to determine whether payloads are ready. The new Publisher uses a single triggering promise that is triggered whenever an AsyncRecord is pushed, and then reset. This may be beneficial as the implementation of `Promise.race` within V8 has a known memory leak for long-running promises. (see https://bugs.chromium.org/p/v8/issues/detail?id=9858). An alternative would be to utilize @brainkim 's memory-safe version detailed in that issue.
1 parent c537db4 commit 9feafd8

File tree

4 files changed

+215
-179
lines changed

4 files changed

+215
-179
lines changed

src/execution/__tests__/defer-test.ts

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -605,11 +605,6 @@ describe('Execute: defer directive', () => {
605605
data: { slowField: 'slow', friends: [{}, {}, {}] },
606606
path: ['hero'],
607607
},
608-
],
609-
hasNext: true,
610-
},
611-
{
612-
incremental: [
613608
{ data: { name: 'Han' }, path: ['hero', 'friends', 0] },
614609
{ data: { name: 'Leia' }, path: ['hero', 'friends', 1] },
615610
{ data: { name: 'C-3PO' }, path: ['hero', 'friends', 2] },
@@ -653,11 +648,6 @@ describe('Execute: defer directive', () => {
653648
},
654649
path: ['hero'],
655650
},
656-
],
657-
hasNext: true,
658-
},
659-
{
660-
incremental: [
661651
{ data: { name: 'Han' }, path: ['hero', 'friends', 0] },
662652
{ data: { name: 'Leia' }, path: ['hero', 'friends', 1] },
663653
{ data: { name: 'C-3PO' }, path: ['hero', 'friends', 2] },

src/execution/__tests__/stream-test.ts

Lines changed: 21 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -151,11 +151,10 @@ describe('Execute: stream directive', () => {
151151
hasNext: true,
152152
},
153153
{
154-
incremental: [{ items: ['banana'], path: ['scalarList', 1] }],
155-
hasNext: true,
156-
},
157-
{
158-
incremental: [{ items: ['coconut'], path: ['scalarList', 2] }],
154+
incremental: [
155+
{ items: ['banana'], path: ['scalarList', 1] },
156+
{ items: ['coconut'], path: ['scalarList', 2] },
157+
],
159158
hasNext: false,
160159
},
161160
]);
@@ -173,15 +172,11 @@ describe('Execute: stream directive', () => {
173172
hasNext: true,
174173
},
175174
{
176-
incremental: [{ items: ['apple'], path: ['scalarList', 0] }],
177-
hasNext: true,
178-
},
179-
{
180-
incremental: [{ items: ['banana'], path: ['scalarList', 1] }],
181-
hasNext: true,
182-
},
183-
{
184-
incremental: [{ items: ['coconut'], path: ['scalarList', 2] }],
175+
incremental: [
176+
{ items: ['apple'], path: ['scalarList', 0] },
177+
{ items: ['banana'], path: ['scalarList', 1] },
178+
{ items: ['coconut'], path: ['scalarList', 2] },
179+
],
185180
hasNext: false,
186181
},
187182
]);
@@ -230,11 +225,6 @@ describe('Execute: stream directive', () => {
230225
path: ['scalarList', 1],
231226
label: 'scalar-stream',
232227
},
233-
],
234-
hasNext: true,
235-
},
236-
{
237-
incremental: [
238228
{
239229
items: ['coconut'],
240230
path: ['scalarList', 2],
@@ -296,11 +286,6 @@ describe('Execute: stream directive', () => {
296286
items: [['banana', 'banana', 'banana']],
297287
path: ['scalarListList', 1],
298288
},
299-
],
300-
hasNext: true,
301-
},
302-
{
303-
incremental: [
304289
{
305290
items: [['coconut', 'coconut', 'coconut']],
306291
path: ['scalarListList', 2],
@@ -379,20 +364,10 @@ describe('Execute: stream directive', () => {
379364
items: [{ name: 'Luke', id: '1' }],
380365
path: ['friendList', 0],
381366
},
382-
],
383-
hasNext: true,
384-
},
385-
{
386-
incremental: [
387367
{
388368
items: [{ name: 'Han', id: '2' }],
389369
path: ['friendList', 1],
390370
},
391-
],
392-
hasNext: true,
393-
},
394-
{
395-
incremental: [
396371
{
397372
items: [{ name: 'Leia', id: '3' }],
398373
path: ['friendList', 2],
@@ -531,11 +506,6 @@ describe('Execute: stream directive', () => {
531506
},
532507
],
533508
},
534-
],
535-
hasNext: true,
536-
},
537-
{
538-
incremental: [
539509
{
540510
items: [{ name: 'Leia', id: '3' }],
541511
path: ['friendList', 2],
@@ -707,7 +677,12 @@ describe('Execute: stream directive', () => {
707677
hasNext: true,
708678
},
709679
},
710-
{ done: false, value: { hasNext: false } },
680+
{
681+
done: false,
682+
value: {
683+
hasNext: false,
684+
},
685+
},
711686
{ done: true, value: undefined },
712687
]);
713688
});
@@ -935,11 +910,6 @@ describe('Execute: stream directive', () => {
935910
},
936911
],
937912
},
938-
],
939-
hasNext: true,
940-
},
941-
{
942-
incremental: [
943913
{
944914
items: [{ nonNullName: 'Han' }],
945915
path: ['friendList', 2],
@@ -984,11 +954,6 @@ describe('Execute: stream directive', () => {
984954
},
985955
],
986956
},
987-
],
988-
hasNext: true,
989-
},
990-
{
991-
incremental: [
992957
{
993958
items: [{ nonNullName: 'Han' }],
994959
path: ['friendList', 2],
@@ -1117,11 +1082,6 @@ describe('Execute: stream directive', () => {
11171082
},
11181083
],
11191084
},
1120-
],
1121-
hasNext: true,
1122-
},
1123-
{
1124-
incremental: [
11251085
{
11261086
items: [{ nonNullName: 'Han' }],
11271087
path: ['friendList', 2],
@@ -1407,9 +1367,6 @@ describe('Execute: stream directive', () => {
14071367
],
14081368
},
14091369
],
1410-
hasNext: true,
1411-
},
1412-
{
14131370
hasNext: false,
14141371
},
14151372
]);
@@ -1556,9 +1513,6 @@ describe('Execute: stream directive', () => {
15561513
path: ['friendList', 2],
15571514
},
15581515
],
1559-
hasNext: true,
1560-
},
1561-
{
15621516
hasNext: false,
15631517
},
15641518
]);
@@ -1612,15 +1566,6 @@ describe('Execute: stream directive', () => {
16121566
data: { scalarField: 'slow', nestedFriendList: [] },
16131567
path: ['nestedObject'],
16141568
},
1615-
],
1616-
hasNext: true,
1617-
},
1618-
done: false,
1619-
});
1620-
const result3 = await iterator.next();
1621-
expectJSON(result3).toDeepEqual({
1622-
value: {
1623-
incremental: [
16241569
{
16251570
items: [{ name: 'Luke' }],
16261571
path: ['nestedObject', 'nestedFriendList', 0],
@@ -1630,8 +1575,8 @@ describe('Execute: stream directive', () => {
16301575
},
16311576
done: false,
16321577
});
1633-
const result4 = await iterator.next();
1634-
expectJSON(result4).toDeepEqual({
1578+
const result3 = await iterator.next();
1579+
expectJSON(result3).toDeepEqual({
16351580
value: {
16361581
incremental: [
16371582
{
@@ -1643,13 +1588,13 @@ describe('Execute: stream directive', () => {
16431588
},
16441589
done: false,
16451590
});
1646-
const result5 = await iterator.next();
1647-
expectJSON(result5).toDeepEqual({
1591+
const result4 = await iterator.next();
1592+
expectJSON(result4).toDeepEqual({
16481593
value: { hasNext: false },
16491594
done: false,
16501595
});
1651-
const result6 = await iterator.next();
1652-
expectJSON(result6).toDeepEqual({
1596+
const result5 = await iterator.next();
1597+
expectJSON(result5).toDeepEqual({
16531598
value: undefined,
16541599
done: true,
16551600
});

0 commit comments

Comments
 (0)