Skip to content

Fix common result merging#177

Merged
vmarkovtsev merged 1 commit intosrc-d:masterfrom
vmarkovtsev:master
Jan 25, 2019
Merged

Fix common result merging#177
vmarkovtsev merged 1 commit intosrc-d:masterfrom
vmarkovtsev:master

Conversation

@vmarkovtsev
Copy link
Collaborator

Signed-off-by: Vadim Markovtsev vadim@sourced.tech

Signed-off-by: Vadim Markovtsev <vadim@sourced.tech>
func mergeMatrices(m1, m2 DenseHistory, granularity1, sampling1, granularity2, sampling2 int,
c1, c2 *core.CommonAnalysisResult) DenseHistory {
commonMerged := *c1
commonMerged := c1.Copy()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is c1 at some point written in parallel? Because if it is, you would still need to wrap the reading and writing in a mutex.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a mutex at the higher level for the rest of the code. This piece expects an immutable object, and I forgot about the mutable maps.

So answering the question: no, it is not, and not intended to be possible.

@vmarkovtsev vmarkovtsev merged commit 6f3f16d into src-d:master Jan 25, 2019
dmytrogajewski pushed a commit to dmytrogajewski/hercules that referenced this pull request Jul 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants