Skip to content

Conversation

@morningman
Copy link
Contributor

@morningman morningman commented Sep 2, 2020

Proposed changes

When creating core local value from CoreDataAllocator,
A lock is needed to protect the modification of _blocks.

Types of changes

  • Bugfix (non-breaking change which fixes an issue)

Checklist

When creating core local value from CoreDataAllocator,
A lock is needed to protect the modification of _blocks.
@morningman morningman self-assigned this Sep 2, 2020
@morningman morningman added the kind/fix Categorizes issue or PR as related to a bug. label Sep 2, 2020
}
private:
static constexpr int ELEMENTS_PER_BLOCK = BLOCK_SIZE / ELEMENT_BYTES;
SpinLock _lock; // lock to protect the modification of _blocks
Copy link
Contributor

Choose a reason for hiding this comment

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

CoreData is to bind data to cpu to eliminate the lock.
If add a lock, it's design is meaningless.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is only used when creating the core local value (by calling get_or_create() of CoreDataAllocatorImpl).
And when creating the value, there is nothing to do with "cpu-bind".

The so called "cpu-bind" operation happens when accessing the CoreLocalValue,
not related to CoreDataAllocatorImpl

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, I have no problem.

}
private:
static constexpr int ELEMENTS_PER_BLOCK = BLOCK_SIZE / ELEMENT_BYTES;
SpinLock _lock; // lock to protect the modification of _blocks
Copy link
Contributor

Choose a reason for hiding this comment

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

OK, I have no problem.

@morningman morningman added the approved Indicates a PR has been approved by one committer. label Sep 3, 2020
@morningman morningman merged commit 15f3e5a into apache:master Sep 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by one committer. kind/fix Categorizes issue or PR as related to a bug.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] BE crash when restarting

3 participants