Skip to content
This repository was archived by the owner on Jul 30, 2025. It is now read-only.

Commit 58de9c0

Browse files
starpitstarpit
committed
fix: avoid excessive status stripe overheads when replaying Notebooks
Fixes #5635
1 parent 5b28dd2 commit 58de9c0

File tree

10 files changed

+130
-44
lines changed

10 files changed

+130
-44
lines changed

‎.travis.yml

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,11 @@ jobs:
7676
- os: osx
7777
env: N="Mac core" CLIENT="default" TEST_FROM_BUILD=`[ "$TRAVIS_OS_NAME" = linux ] && echo "${TRAVIS_BUILD_DIR}/dist/electron/Kui-linux-x64/Kui" || echo "${TRAVIS_BUILD_DIR}/dist/electron/Kui-darwin-x64/Kui.app/Contents/MacOS/Kui"` HOMEBREW_NO_AUTO_UPDATE=1 NEEDS_MINIO=true
7878
before_install: which jq || brew install jq
79-
script: concurrently -n CORE,SUP1,EDIT,SUP2 'npm run test1 core' 'npm run test2 core-support' 'npm run test3 editor' 'npm run test4 core-support2'
79+
script:
80+
- npm run test1 core
81+
- npm run test2 core-support
82+
- npm run test3 editor
83+
- npm run test4 core-support2
8084

8185
# CLIENT=default | os=Linux | targets=webpack | core, core-support, editor, core-support2
8286
- env: N="Webpack core" CLIENT="default" MOCHA_RUN_TARGET="webpack" KUI_USE_PROXY="true" NEEDS_MINIO=true

‎packages/core/src/core/events.ts

Lines changed: 32 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -90,36 +90,44 @@ class WriteEventBus extends EventBusBase {
9090
setTimeout(() => this.eventBus.emit(`/tab/layout/change/${tabUUID}`, evt))
9191
}
9292

93-
private emitCommandEvent(which: 'start' | 'complete', event: CommandStartEvent | CommandCompleteEvent) {
93+
private emitCommandEvent(
94+
which: 'start' | 'complete',
95+
event: CommandStartEvent | CommandCompleteEvent,
96+
isReplay: boolean
97+
) {
9498
this.eventBus.emit(`/command/${which}`, event)
9599

96100
if (event.execType !== ExecType.Nested) {
97-
this.eventBus.emit(`/command/${which}/fromuser`, event)
98-
this.eventBus.emit(`/command/${which}/fromuser/${event.tab.uuid}`, event)
101+
const from = isReplay ? 'replay' : 'fromuser'
102+
103+
this.eventBus.emit(`/command/${which}/${from}`, event)
104+
this.eventBus.emit(`/command/${which}/${from}/${event.tab.uuid}`, event)
99105

100106
const primary = getPrimaryTabId(event.tab)
101107
if (event.tab.uuid !== primary) {
102-
this.eventBus.emit(`/command/${which}/fromuser/${primary}`, event)
108+
this.eventBus.emit(`/command/${which}/${from}/${primary}`, event)
103109
}
104110

105-
this.eventBus.emit(`/command/${which}/fromuser/${primary}/type/${event.execType}`, event)
111+
this.eventBus.emit(`/command/${which}/${from}/${primary}/type/${event.execType}`, event)
106112
}
107113
}
108114

109-
public emitCommandStart(event: CommandStartEvent): void {
110-
this.emitCommandEvent('start', event)
115+
public emitCommandStart(event: CommandStartEvent, isReplay = false): void {
116+
this.emitCommandEvent('start', event, isReplay)
111117
}
112118

113-
public emitCommandComplete(event: CommandCompleteEvent): void {
114-
this.emitCommandEvent('complete', event)
119+
public emitCommandComplete(event: CommandCompleteEvent, isReplay = false): void {
120+
this.emitCommandEvent('complete', event, isReplay)
115121

116122
if (event.execType !== ExecType.Nested) {
117-
this.eventBus.emit(`/command/complete/fromuser/${event.responseType}`, event)
118-
this.eventBus.emit(`/command/complete/fromuser/${event.responseType}/${event.tab.uuid}`, event)
123+
const from = isReplay ? 'replay' : 'fromuser'
124+
125+
this.eventBus.emit(`/command/complete/${from}/${event.responseType}`, event)
126+
this.eventBus.emit(`/command/complete/${from}/${event.responseType}/${event.tab.uuid}`, event)
119127

120128
const primary = getPrimaryTabId(event.tab)
121129
if (primary !== event.tab.uuid) {
122-
this.eventBus.emit(`/command/complete/fromuser/${event.responseType}/${primary}`, event)
130+
this.eventBus.emit(`/command/complete/${from}/${event.responseType}/${primary}`, event)
123131
}
124132
}
125133
}
@@ -212,17 +220,19 @@ class ReadEventBus extends WriteEventBus {
212220
this.eventBus.off(`/tab/layout/change/${tabUUID}`, listener)
213221
}
214222

215-
private onCommand(
223+
private onCommand<Handler extends CommandStartHandler | CommandCompleteHandler>(
216224
which: 'start' | 'complete',
217225
splitId: string,
218-
splitHandler: CommandStartHandler | CommandCompleteHandler,
226+
splitHandler: Handler,
219227
tabId?: string,
220228
tabHandler = splitHandler
221229
): void {
222230
this.eventBus.on(`/command/${which}/fromuser/${splitId}`, splitHandler)
231+
this.eventBus.on(`/command/${which}/replay/${splitId}`, splitHandler)
223232

224233
if (tabId) {
225234
this.eventBus.on(`/command/${which}/fromuser/${tabId}/type/${ExecType.ClickHandler}`, tabHandler)
235+
this.eventBus.on(`/command/${which}/replay/${tabId}/type/${ExecType.ClickHandler}`, tabHandler)
226236
}
227237
}
228238

@@ -248,11 +258,11 @@ class ReadEventBus extends WriteEventBus {
248258
this.eventBus.off('/command/start/fromuser', handler)
249259
}
250260

251-
public onAnyCommandComplete(handler: CommandStartHandler | (() => void)) {
261+
public onAnyCommandComplete(handler: CommandCompleteHandler | (() => void)) {
252262
this.eventBus.on('/command/complete/fromuser', handler)
253263
}
254264

255-
public offAnyCommandComplete(handler: CommandStartHandler | (() => void)) {
265+
public offAnyCommandComplete(handler: CommandCompleteHandler | (() => void)) {
256266
this.eventBus.off('/command/complete/fromuser', handler)
257267
}
258268

@@ -278,8 +288,14 @@ class ReadEventBus extends WriteEventBus {
278288
): void {
279289
this.eventBus.on(`/command/complete/fromuser/${responseType}/${splitId}`, splitHandler)
280290

291+
// if you don't want the sidecar to open on replay, comment this out:
292+
this.eventBus.on(`/command/complete/replay/${responseType}/${splitId}`, splitHandler)
293+
281294
if (tabId) {
282295
this.eventBus.on(`/command/complete/fromuser/${responseType}/${tabId}`, tabHandler)
296+
297+
// if you don't want the sidecar to open on replay, comment this out:
298+
this.eventBus.on(`/command/complete/replay/${responseType}/${tabId}`, tabHandler)
283299
}
284300
}
285301

‎plugins/plugin-core-support/src/lib/cmds/replay.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -140,13 +140,13 @@ function withNewTabUUID(tab: Tab, uuid: string) {
140140
/** Re-emit prior start event in new tab */
141141
function reEmitStartInTab(tab: Tab, uuid: string, startEvent: SnapshotBlock['startEvent']) {
142142
const evt = Object.assign({}, startEvent, withNewTabUUID(tab, uuid))
143-
eventBus.emitCommandStart(evt)
143+
eventBus.emitCommandStart(evt, true)
144144
}
145145

146146
/** Re-emit prior complete event in new tab */
147147
function reEmitCompleteInTab(tab: Tab, uuid: string, completeEvent: SnapshotBlock['completeEvent']) {
148148
const evt = Object.assign({}, completeEvent, withNewTabUUID(tab, uuid))
149-
eventBus.emitCommandComplete(evt)
149+
eventBus.emitCommandComplete(evt, true)
150150
}
151151

152152
/** Re-execute command line in new tab */

‎plugins/plugin-core-support/src/test/core-support/replay.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ describe(`snapshot and replay ${process.env.MOCHA_RUN_TARGET || ''}`, function(t
5151
await this.app.client.waitUntil(async () => {
5252
const txt = await this.app.client.getText(Selectors.OUTPUT_N(count))
5353
if (++idx > 5) {
54-
console.error(`still waiting for expected=${txt}; actual=${txt}`)
54+
console.error(`still waiting for expected=${base64Output}; actual=${txt}`)
5555
}
5656
return txt === base64Output
5757
}, CLI.waitTimeout)

‎plugins/plugin-core-support/src/test/core-support2/history.ts

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2017 IBM Corporation
2+
* Copyright 2017,2020 IBM Corporation
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -30,15 +30,20 @@ describe('command history plain', function(this: Common.ISuite) {
3030
it('should list local files', () =>
3131
CLI.command(listCommand, this.app)
3232
.then(ReplExpect.okWith('README.md'))
33-
.catch(Common.oops(this)))
33+
.catch(Common.oops(this, true)))
3434

3535
it('should hit the up arrow and see previous command', async () => {
3636
try {
3737
await this.app.client.keys(Keys.ctrlP)
38+
39+
let idx = 0
3840
await this.app.client.waitUntil(async () => {
3941
const promptValue = await this.app.client.getValue(Selectors.PROMPT_FINAL)
42+
if (++idx > 5) {
43+
console.error(`still waiting for expectedPromptValue=${listCommand}; actualPromptValue=${promptValue}`)
44+
}
4045
return promptValue === listCommand
41-
})
46+
}, CLI.waitTimeout)
4247
} catch (err) {
4348
await Common.oops(this, true)(err)
4449
}
@@ -47,10 +52,15 @@ describe('command history plain', function(this: Common.ISuite) {
4752
it('should hit the down arrow and see previous command', async () => {
4853
try {
4954
await this.app.client.keys(Keys.ctrlN)
55+
56+
let idx = 0
5057
await this.app.client.waitUntil(async () => {
5158
const promptValue = await this.app.client.getValue(Selectors.PROMPT_FINAL)
59+
if (++idx > 5) {
60+
console.error(`still waiting for empty prompt; actualPromptValue=${promptValue}`)
61+
}
5262
return promptValue.length === 0
53-
})
63+
}, CLI.waitTimeout)
5464
} catch (err) {
5565
await Common.oops(this, true)(err)
5666
}
@@ -60,18 +70,18 @@ describe('command history plain', function(this: Common.ISuite) {
6070
it(`should list history with filter 1`, () =>
6171
CLI.command(`history 1 lls`, this.app)
6272
.then(ReplExpect.okWithOnly(listCommand))
63-
.catch(Common.oops(this)))
73+
.catch(Common.oops(this, true)))
6474

6575
it(`should list history 2 and show the list command`, () =>
6676
CLI.command(`history 2`, this.app)
6777
.then(ReplExpect.okWith(listCommand))
68-
.catch(Common.oops(this)))
78+
.catch(Common.oops(this, true)))
6979

7080
// get something on the screen
7181
it(`should list local files again`, () =>
7282
CLI.command(listCommand, this.app)
7383
.then(ReplExpect.okWith('README.md'))
74-
.catch(Common.oops(this)))
84+
.catch(Common.oops(this, true)))
7585

7686
it('should re-execte from history via mouse click', async () => {
7787
try {
@@ -91,30 +101,30 @@ describe('command history plain', function(this: Common.ISuite) {
91101
it(`should list history with filter, expect nothing`, () =>
92102
CLI.command(`history gumbogumbo`, this.app)
93103
.then(ReplExpect.justOK) // some random string that won't be in the command history
94-
.catch(Common.oops(this)))
104+
.catch(Common.oops(this, true)))
95105

96106
it(`should delete command history`, () =>
97107
CLI.command(`history -c`, this.app)
98108
.then(ReplExpect.justOK)
99-
.catch(Common.oops(this)))
109+
.catch(Common.oops(this, true)))
100110

101111
it(`should list history with no args after delete and expect nothing`, () =>
102112
CLI.command(`history`, this.app)
103113
.then(ReplExpect.justOK)
104-
.catch(Common.oops(this)))
114+
.catch(Common.oops(this, true)))
105115

106116
it(`should list history with idx arg after delete and expect only the previous`, () =>
107117
CLI.command(`history 10`, this.app)
108118
.then(ReplExpect.okWithOnly('history'))
109-
.catch(Common.oops(this)))
119+
.catch(Common.oops(this, true)))
110120

111121
it(`should delete command history again`, () =>
112122
CLI.command(`history -c`, this.app)
113123
.then(ReplExpect.justOK)
114-
.catch(Common.oops(this)))
124+
.catch(Common.oops(this, true)))
115125

116126
it(`should list history with idx and filter args after delete and expect nothing`, () =>
117127
CLI.command(`history 10 lls`, this.app)
118128
.then(ReplExpect.justOK) // some random string that won't be in the command history
119-
.catch(Common.oops(this)))
129+
.catch(Common.oops(this, true)))
120130
})

‎plugins/plugin-core-support/src/test/core-support2/split-terminals.ts

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -99,35 +99,50 @@ describe(`split terminals created split check ${process.env.MOCHA_RUN_TARGET ||
9999
verifyBlockCount.is(1) // the "created split" message should be gone!
100100
})
101101

102-
describe(`split terminals ${process.env.MOCHA_RUN_TARGET || ''}`, function(this: Common.ISuite) {
102+
describe(`split terminals close all ${process.env.MOCHA_RUN_TARGET || ''}`, function(this: Common.ISuite) {
103103
before(Common.before(this))
104104
after(Common.after(this))
105105
Util.closeAllExceptFirstTab.bind(this)()
106106

107-
const showVersion = version.bind(this)
108-
const splitTheTerminalViaButton = splitViaButton.bind(this)
109107
const splitTheTerminalViaCommand = splitViaCommand.bind(this)
110-
const closeTheSplit = close.bind(this)
111-
const focusOnSplit = focusAndValidate.bind(this)
112108
const count = expectSplits.bind(this)
113-
const cd = changeDir.bind(this)
114-
const cwdIs = inDir.bind(this)
115-
116-
// here come the tests
109+
const showVersion = version.bind(this)
117110

118111
it('should create a new tab via command', () =>
119112
CLI.command('tab new', this.app)
120113
.then(() => this.app.client.waitForVisible(Selectors.TAB_SELECTED_N(2)))
121114
.then(() => CLI.waitForSession(this)) // should have an active repl
122115
.catch(Common.oops(this, true)))
116+
123117
splitTheTerminalViaCommand(2)
118+
124119
count(2)
120+
125121
it('should close that new tab entirely, i.e. all splits plus the tab should be closed', () =>
126122
CLI.command('tab close -A', this.app)
127123
.then(() => this.app.client.waitForExist(Selectors.TAB_N(2), 5000, true))
128124
.then(() => this.app.client.waitForVisible(Selectors.TAB_SELECTED_N(1)))
129125
.catch(Common.oops(this, true)))
130126

127+
showVersion(1)
128+
})
129+
130+
describe(`split terminals general ${process.env.MOCHA_RUN_TARGET || ''}`, function(this: Common.ISuite) {
131+
before(Common.before(this))
132+
after(Common.after(this))
133+
Util.closeAllExceptFirstTab.bind(this)()
134+
135+
const showVersion = version.bind(this)
136+
const splitTheTerminalViaButton = splitViaButton.bind(this)
137+
const splitTheTerminalViaCommand = splitViaCommand.bind(this)
138+
const closeTheSplit = close.bind(this)
139+
const focusOnSplit = focusAndValidate.bind(this)
140+
const count = expectSplits.bind(this)
141+
const cd = changeDir.bind(this)
142+
const cwdIs = inDir.bind(this)
143+
144+
// here come the tests
145+
131146
const { fullpath: dir1, clean: clean1 } = dir('aaa')
132147
const { fullpath: dir2, clean: clean2 } = dir('bbb')
133148

‎plugins/plugin-git/src/CurrentGitBranch.tsx

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,16 @@ export default class CurrentGitBranch extends React.PureComponent<Props, State>
4343
}
4444
}
4545

46+
/** Avoid recomputation for a flurry of events */
47+
private last: number
48+
private debounce(): boolean {
49+
const now = Date.now()
50+
const last = this.last
51+
this.last = now
52+
53+
return last && now - last < 250
54+
}
55+
4656
/**
4757
* Check the current branch, and the dirtiness thereof.
4858
*
@@ -51,6 +61,8 @@ export default class CurrentGitBranch extends React.PureComponent<Props, State>
5161
const tab = getCurrentTab()
5262
if (!tab || !tab.REPL) {
5363
return
64+
} else if (this.debounce()) {
65+
return
5466
}
5567

5668
try {
@@ -71,7 +83,8 @@ export default class CurrentGitBranch extends React.PureComponent<Props, State>
7183
})
7284
} catch (error) {
7385
const err = error as CodedError
74-
if (err.code !== 128 && !/ambiguous argument 'HEAD'/.test(err.message)) {
86+
this.last = undefined
87+
if (err.code !== 128 && !/ambiguous argument 'HEAD'/.test(err.message) && !/not a git repo/.test(err.message)) {
7588
// 128: not a git repository; don't report those as errors
7689
console.error('unable to determine git branch', err.code, typeof err.code, err)
7790
}

‎plugins/plugin-kubectl/components/src/CurrentContext.tsx

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,13 +78,25 @@ export default class CurrentContext extends React.PureComponent<{}, State> {
7878
return context
7979
}
8080

81+
/** Avoid recomputation for a flurry of events */
82+
private last: number
83+
private debounce(): boolean {
84+
const now = Date.now()
85+
const last = this.last
86+
this.last = now
87+
88+
return last && now - last < 250
89+
}
90+
8191
private async reportCurrentContext(idx?: Tab | number) {
8292
const tab = getTab(idx)
8393
if (!tab || !tab.REPL) {
8494
if (tab && !tab.REPL) {
8595
eventChannelUnsafe.once(`/tab/new/${tab.uuid}`, () => this.reportCurrentContext())
8696
}
8797
return
98+
} else if (this.debounce()) {
99+
return
88100
}
89101

90102
try {
@@ -95,6 +107,7 @@ export default class CurrentContext extends React.PureComponent<{}, State> {
95107
})
96108
} catch (err) {
97109
console.error(err)
110+
this.last = undefined
98111
this.setState({
99112
text: '',
100113
viewLevel: 'hidden'

0 commit comments

Comments
 (0)