suggestions for PR 910

[AlexHentschel]

Implements suggestions for PR #910:

• It would be great to fix this performance bottleneck:

  flow-go/engine/common/synchronization/engine.go

  Line 489 in 5fd2ff2

  block, err := e.blocks.ByHeight(height)

  • In contrast to badger.Headers, which has a cache for heights, badger.blocks does not have such a cache and instead always hits the data base.
• I think we could inline method

  flow-go/engine/common/synchronization/engine.go

  Lines 581 to 596 in 5fd2ff2

  	// processIncoming processes an incoming block, so we can take into account the
  	// overlap between block IDs and heights.
  	func (e *Engine) processIncomingBlock(originID flow.Identifier, block *flow.Block) {
  	shouldProcess := e.core.HandleBlock(block.Header)
  	if !shouldProcess {
  	return
  	}
  	synced := &events.SyncedBlock{
  	OriginID: originID,
  	Block: block,
  	}
  	e.comp.SubmitLocal(synced)
  	}

  as it is only used once.
• We might be able to reduce lock congestion in Core. HandleHeight :
  • we always lock right away, but on the happy path when the node is up to date, this code would return right away:

    flow-go/module/synchronization/core.go

    Lines 102 to 105 in 5fd2ff2

    	// don't bother queueing anything if we're within tolerance
    	if c.WithinTolerance(final, height) {
    	return
    	}

    . WithinTolerance is fully concurrency safe without any locks (we also call it externally, here). Hence, we could move the lock further down into the if statement, where we actually update Core's state.