Skip to content

Commit f3c8bd9

Browse files
pattyshackpattyshack
committed
Change View merging to operate on execution snapshot
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.
1 parent 34ff8f0 commit f3c8bd9

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)