Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(feat) Impl SegmentList to replace ArrayDequeue in LogManagerImpl, #335 #377

Merged
merged 7 commits into from
Dec 30, 2019

Conversation

killme2008
Copy link
Contributor

@killme2008 killme2008 commented Dec 25, 2019

Motivation:

See #335

Modification:

  • Introduce a new data structure SegmentList that is a list of segment, every segment is an array contains 128 elements, and only supports removing elements from start or end:
                            segments
       /                        |                       \
segment                      segment                      segment 
[0, 1 ...  127]             [128, 129 ... 255]          [256, 1 ... 383] 

The removeFromFirstWhen and removeFromLastWhen methods accept a predicate and delete the elements from start or end until the predicate returns false. The remove operation only set the elements to be null and remeber the deleted poistion,do not have any array elements movement at all. If a segment is empty after remove, it will be removed from segment list too.
Also, I use the Recyclable to reuse the Segment instances.

The SegmentList performance in benchmark is alaways better than ArrayDequeue in SegmentListTest#simpleBenchmark, at most 10% improvement(The benchmark has some random factor).

  • Replace ArrayDeque in LogManagerImpl with SegmentList
  • Fixed deadlock in RepeatedTimer#destroy and RepeatedTimer#run

Result:

Fixes #335

@killme2008 killme2008 requested review from fengjiachun and SteNicholas and removed request for fengjiachun December 25, 2019 13:39
*/
public class SegmentList<T> {
private static final int SEGMENT_SHIFT = 8;
public static final int SEGMENT_SIZE = 2 << (SEGMENT_SHIFT - 1);
Copy link
Contributor

Choose a reason for hiding this comment

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

every segment contains at most 128 elements, but SEGMENT_SIZE 2 << 7 equals 256.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed. 128 is a balance between time and space.


@SuppressWarnings("unchecked")
public void addAll(final Collection<T> coll) {
Object[] src = coll.toArray();
Copy link
Contributor

@fengjiachun fengjiachun Dec 26, 2019

Choose a reason for hiding this comment

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

入参为 Collection 类型更通用,原有 ArrayDeque 的设计也没有避免一次 toArray 的多余 memcopy

但我还是建议这里针对 ArrayList 做一个 fast path 优化,在入参 coll 为 ArrayList 类型且 UnsafeUtil.hasUnsafe() == true 的情况下通过 UnsafeReferenceFieldUpdater 直接拿到 ArrayList 中的 object[],用于后面的 copy,这样节省了一次 memcopy;

AppendEntries() 是 jraft 中的热点执行路径,会调用 addAll () 来 append log,所以我认为即使代码丑一点,但是优化是值得的,理论上会减少大量的 toArray 带来的临时 'object[]' 的创建和 memcopy

Copy link
Contributor Author

Choose a reason for hiding this comment

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

嗯,搞了下,测试下来,稳定提升在 15% 以上 👍 ,不过需要将 UnsafeReferenceFieldUpdater 从 rheakv 拷贝一份到 jraft-core 。不做 move,因为不确定外面有没有人依赖了。

@fengjiachun
Copy link
Contributor

                            segments
       /                        |                       \
segment                      segment                      segment 
[0, 1 ...  127]             [128, 129 ... 255]          [256, 1 ... 383] 

这个图是不是可以加到 SegmentList 的类注释里面

@killme2008
Copy link
Contributor Author

Updated:

  1. SegmentList constructor accepts boolean recycleSegment to choose whether to enable recycling segment.
  2. SegmentList#addAll adds a fast path for ArrayList, suggested by @fengjiachun
  3. Replace pendingMetaQueue in BallotBox with SegmentList too.
  4. Move some utilities from rheakv-core to jraft-core, such as ReferenceFieldUpdater etc.

@fengjiachun fengjiachun merged commit 64daa8b into master Dec 30, 2019
@fengjiachun fengjiachun deleted the feature/segment-list branch December 30, 2019 02:22
@fengjiachun fengjiachun mentioned this pull request Apr 17, 2020
12 tasks
zongtanghu pushed a commit that referenced this pull request Apr 28, 2020
… (#377)

* (feat) Impl SegmentList to replace ArrayDequeue in LogManagerImpl, #335

* (fix) comments

* (feat) MockStateMachine prevents log duplication

* (fix) set SEGMENT_SHIFT=7

* (feat) minor changes, replace ArrayDeque in BallotBox

* (fix) format

* (fix) NPE when unsafe not supported
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.

logsInMemory needs a more appropriate deque
4 participants