Skip to content

Commit 77a2863

Browse files
bors[bot]pattyshack
bors[bot]
and
pattyshack
authored
Merge #3975
3975: Change View merging to operate on execution snapshot r=pattyshack a=pattyshack Note that this change is slightly less efficient than the original implementation since UpdatedRegisters does extra/unnecessary sorting. The sorting is only needed for ledger updates, we should move aways from sorting the updates in general. Co-authored-by: Patrick Lee <[email protected]>
2 parents 34ff8f0 + f3c8bd9 commit 77a2863

File tree

12 files changed

+157
-119
lines changed

12 files changed

+157
-119
lines changed

‎engine/execution/computation/computer/computer.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -529,5 +529,5 @@ func (e *blockComputer) mergeView(
529529
mergeSpan := e.tracer.StartSpanFromParent(parentSpan, mergeSpanName)
530530
defer mergeSpan.End()
531531

532-
return parent.MergeView(child)
532+
return parent.Merge(child)
533533
}

‎engine/execution/state/delta/view.go

Lines changed: 18 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"sync"
66

77
"github.com/onflow/flow-go/crypto/hash"
8+
"github.com/onflow/flow-go/fvm/meter"
89
"github.com/onflow/flow-go/fvm/state"
910
"github.com/onflow/flow-go/model/flow"
1011
)
@@ -16,7 +17,6 @@ import (
1617
type View struct {
1718
delta Delta
1819
regTouchSet map[flow.RegisterID]struct{} // contains all the registers that have been touched (either read or written to)
19-
readsCount uint64 // contains the total number of reads
2020
// spockSecret keeps the secret used for SPoCKs
2121
// TODO we can add a flag to disable capturing spockSecret
2222
// for views other than collection views to improve performance
@@ -124,8 +124,13 @@ func (v *View) NewChild() state.View {
124124
return NewDeltaView(state.NewPeekerStorageSnapshot(v))
125125
}
126126

127-
func (v *View) DropDelta() {
127+
func (v *View) Meter() *meter.Meter {
128+
return nil
129+
}
130+
131+
func (v *View) DropChanges() error {
128132
v.delta = NewDelta()
133+
return nil
129134
}
130135

131136
func (v *View) AllRegisterIDs() []flow.RegisterID {
@@ -158,7 +163,6 @@ func (v *View) Get(registerID flow.RegisterID) (flow.RegisterValue, error) {
158163
// capture register touch
159164
v.regTouchSet[registerID] = struct{}{}
160165
// increase reads
161-
v.readsCount++
162166
}
163167
// every time we read a value (order preserving) we update the secret
164168
// with the registerID only (value is not required)
@@ -206,33 +210,24 @@ func (v *View) Delta() Delta {
206210
return v.delta
207211
}
208212

209-
// MergeView applies the changes from a the given view to this view.
210-
// TODO rename this, this is not actually a merge as we can't merge
211-
// readFunc s.
212-
213-
func (v *View) MergeView(ch state.View) error {
214-
215-
child, ok := ch.(*View)
216-
if !ok {
217-
return fmt.Errorf("can not merge view: view type mismatch (given: %T, expected:delta.View)", ch)
218-
}
213+
// TODO(patrick): remove after updating emulator
214+
func (view *View) MergeView(child state.ExecutionSnapshot) error {
215+
return view.Merge(child)
216+
}
219217

220-
for id := range child.Interactions().RegisterTouches() {
221-
v.regTouchSet[id] = struct{}{}
218+
func (view *View) Merge(child state.ExecutionSnapshot) error {
219+
for _, id := range child.AllRegisterIDs() {
220+
view.regTouchSet[id] = struct{}{}
222221
}
223222

224-
// SpockSecret is order aware
225-
// TODO return the error and handle it properly on other places
226-
227-
spockSecret := child.SpockSecret()
228-
229-
_, err := v.spockSecretHasher.Write(spockSecret)
223+
_, err := view.spockSecretHasher.Write(child.SpockSecret())
230224
if err != nil {
231225
return fmt.Errorf("merging SPoCK secrets failed: %w", err)
232226
}
233-
v.delta.MergeWith(child.delta)
234227

235-
v.readsCount += child.readsCount
228+
for _, entry := range child.UpdatedRegisters() {
229+
view.delta.Data[entry.Key] = entry.Value
230+
}
236231

237232
return nil
238233
}
@@ -246,11 +241,6 @@ func (r *Snapshot) RegisterTouches() map[flow.RegisterID]struct{} {
246241
return ret
247242
}
248243

249-
// ReadsCount returns the total number of reads performed on this view including all child views
250-
func (v *View) ReadsCount() uint64 {
251-
return v.readsCount
252-
}
253-
254244
// SpockSecret returns the secret value for SPoCK
255245
//
256246
// This function modifies the internal state of the SPoCK secret hasher.

‎engine/execution/state/delta/view_test.go

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ func TestViewSet(t *testing.T) {
150150
})
151151
}
152152

153-
func TestViewMergeView(t *testing.T) {
153+
func TestViewMerge(t *testing.T) {
154154
registerID1 := flow.NewRegisterID("fruit", "")
155155
registerID2 := flow.NewRegisterID("vegetable", "")
156156
registerID3 := flow.NewRegisterID("diary", "")
@@ -164,7 +164,7 @@ func TestViewMergeView(t *testing.T) {
164164
err = chView.Set(registerID2, flow.RegisterValue("carrot"))
165165
assert.NoError(t, err)
166166

167-
err = v.MergeView(chView)
167+
err = v.Merge(chView)
168168
assert.NoError(t, err)
169169

170170
b1, err := v.Get(registerID1)
@@ -185,7 +185,7 @@ func TestViewMergeView(t *testing.T) {
185185
assert.NoError(t, err)
186186

187187
chView := v.NewChild()
188-
err = v.MergeView(chView)
188+
err = v.Merge(chView)
189189
assert.NoError(t, err)
190190

191191
b1, err := v.Get(registerID1)
@@ -207,7 +207,7 @@ func TestViewMergeView(t *testing.T) {
207207
err = chView.Set(registerID2, flow.RegisterValue("carrot"))
208208
assert.NoError(t, err)
209209

210-
err = v.MergeView(chView)
210+
err = v.Merge(chView)
211211
assert.NoError(t, err)
212212

213213
b1, err := v.Get(registerID1)
@@ -228,7 +228,7 @@ func TestViewMergeView(t *testing.T) {
228228
chView := v.NewChild()
229229
err = chView.Set(registerID1, flow.RegisterValue("orange"))
230230
assert.NoError(t, err)
231-
err = v.MergeView(chView)
231+
err = v.Merge(chView)
232232
assert.NoError(t, err)
233233

234234
b, err := v.Get(registerID1)
@@ -245,7 +245,7 @@ func TestViewMergeView(t *testing.T) {
245245
chView := v.NewChild()
246246
err = chView.Set(registerID1, flow.RegisterValue("orange"))
247247
assert.NoError(t, err)
248-
err = v.MergeView(chView)
248+
err = v.Merge(chView)
249249
assert.NoError(t, err)
250250

251251
b, err := v.Get(registerID1)
@@ -276,7 +276,7 @@ func TestViewMergeView(t *testing.T) {
276276
hash2 := expSpock2.SumHash()
277277
assert.Equal(t, chView.(*delta.View).SpockSecret(), []uint8(hash2))
278278

279-
err = v.MergeView(chView)
279+
err = v.Merge(chView)
280280
assert.NoError(t, err)
281281

282282
hashIt(t, expSpock1, hash2)
@@ -295,7 +295,7 @@ func TestViewMergeView(t *testing.T) {
295295
err = chView.Set(registerID3, flow.RegisterValue("milk"))
296296
assert.NoError(t, err)
297297

298-
err = v.MergeView(chView)
298+
err = v.Merge(chView)
299299
assert.NoError(t, err)
300300

301301
reads := v.Interactions().Reads
@@ -405,7 +405,7 @@ func TestView_AllRegisterIDs(t *testing.T) {
405405
err = vv.Set(idF, flow.RegisterValue("f_value"))
406406
assert.NoError(t, err)
407407

408-
err = v.MergeView(vv)
408+
err = v.Merge(vv)
409409
assert.NoError(t, err)
410410
allRegs := v.Interactions().AllRegisterIDs()
411411
assert.Len(t, allRegs, 6)

‎fvm/environment/programs_test.go

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ func Test_Programs(t *testing.T) {
235235
txAView = viewExecA
236236

237237
// merge it back
238-
err = mainView.MergeView(viewExecA)
238+
err = mainView.Merge(viewExecA)
239239
require.NoError(t, err)
240240

241241
// execute transaction again, this time make sure it doesn't load code
@@ -264,7 +264,7 @@ func Test_Programs(t *testing.T) {
264264
compareViews(t, viewExecA, viewExecA2)
265265

266266
// merge it back
267-
err = mainView.MergeView(viewExecA2)
267+
err = mainView.Merge(viewExecA2)
268268
require.NoError(t, err)
269269
})
270270

@@ -349,7 +349,7 @@ func Test_Programs(t *testing.T) {
349349
contractBView = deltaB
350350

351351
// merge it back
352-
err = mainView.MergeView(viewExecB)
352+
err = mainView.Merge(viewExecB)
353353
require.NoError(t, err)
354354

355355
// rerun transaction
@@ -382,7 +382,7 @@ func Test_Programs(t *testing.T) {
382382
compareViews(t, viewExecB, viewExecB2)
383383

384384
// merge it back
385-
err = mainView.MergeView(viewExecB2)
385+
err = mainView.Merge(viewExecB2)
386386
require.NoError(t, err)
387387
})
388388

@@ -413,7 +413,7 @@ func Test_Programs(t *testing.T) {
413413
compareViews(t, txAView, viewExecA)
414414

415415
// merge it back
416-
err = mainView.MergeView(viewExecA)
416+
err = mainView.Merge(viewExecA)
417417
require.NoError(t, err)
418418
})
419419

@@ -488,6 +488,5 @@ func Test_Programs(t *testing.T) {
488488
func compareViews(t *testing.T, a, b *delta.View) {
489489
require.Equal(t, a.Delta(), b.Delta())
490490
require.Equal(t, a.Interactions(), b.Interactions())
491-
require.Equal(t, a.ReadsCount(), b.ReadsCount())
492491
require.Equal(t, a.SpockSecret(), b.SpockSecret())
493492
}

0 commit comments

Comments
 (0)