Use CopyOnWriteArrayList instead of recreating manually

[kezhenxu94]

Please answer these questions before submitting pull request

• Why submit this pull request?
• Bug fix
• New feature provided
• Improve performance

skywalking/apm-commons/apm-datacarrier/src/main/java/org/apache/skywalking/apm/commons/datacarrier/consumer/MultipleChannelsConsumer.java

Lines 99 to 109 in 8faa356

	public void addNewTarget(Channels channels, IConsumer consumer) {
	Group group = new Group(channels, consumer);
	// Recreate the new list to avoid change list while the list is used in consuming.
	ArrayList<Group> newList = new ArrayList<Group>();
	for (Group target : consumeTargets) {
	newList.add(target);
	}
	newList.add(group);
	consumeTargets = newList;
	size += channels.size();
	}

the comment // Recreate the new list to avoid change list while the list is used in consuming. reminds me of the CopyOnWrite data structure that is perfectly suitable to this scenario, although the method addNewTarget is not frequently invoked, I did a simple benchmark out of curiosity.

This patch also fixes that the non-atomic operation += on the volatile variable size

[kezhenxu94]

FYI @wu-sheng , I'm recently considering whether we should create a new module/directory to place all the benchmark tests like other projects, the reasons are:

1. we have some benchmarks that are independent to any existed module, such as LinkedArrayBenchmark, StringFormatBenchmarkTest, IMHO they should be placed in an independent module, otherwise it's hard for new-comers to refer to and find out whether we already have that benchmark or not

2. some of the existed benchmarks are annotated with @Test, and are executed in maven test phase (I think), which is not necessary and time consuming, gathering them together may be easy to manage and remove @Test

Just a bold thought and may make no sense at all :)

[wu-sheng]

@kezhenxu94 How about we create WIKI pages for that? And link to the PRs?

[wu-sheng]

We have this. https://cwiki.apache.org/confluence/display/SKYWALKING/Home

I could grant committers access right. Then all the acceptable performance improvement PRs, should have a benchmark list there.

[kezhenxu94]

@kezhenxu94 How about we create WIKI pages for that? And link to the PRs?

That's a great alternative, it would be easy for new-comers who are interested in performance optimization and can refer to the wiki

[dmsolr]

Out of this patch.
ArrayList has worse performance because ArrayList uses add() one by one. It would be better if using addAll() or new ArrayList<>(collections).

[wu-sheng]

Is this really performance concern? This never changes in runtime, right?

[wu-sheng]

We accept a lot of performance improvements these days, but we don't need to focus on that too much.

[wu-sheng]

Like I said in some other PRs, we don't require people to change their coding style, due to that is personal interests.

It is only acceptable when causing runtime performance issue.

[dmsolr]

Yes. In this case, it is more importance to fix size. And I say that it is out of this issue. Just for talking.