restored non parallel-CP-execution #14
Conversation
There was a problem hiding this comment.
The following PRs sychronize the ContentModel's set method:
DantaFramework/AEM#12
DantaFramework/JahiaDF#11
Is it really still desired to disable parallel CP execution?
| @@ -56,7 +56,7 @@ | |||
| @Override | |||
| public List<String> execute(ExecutionContext executionContext, ContentModel contentModel) | |||
| throws AcceptsException, ProcessException { | |||
| List<String> currentProcessorChain = Collections.synchronizedList(new ArrayList<>()); | |||
| List<String> currentProcessorChain = new ArrayList<>(); | |||
There was a problem hiding this comment.
I would undo this change. It has negligible impact to leave it in place for serial CP execution, but removing it makes it more likely that it will be forgotten in the future when parallel execution is reintroduced.
| @@ -56,7 +56,7 @@ | |||
| @Override | |||
| public List<String> execute(ExecutionContext executionContext, ContentModel contentModel) | |||
| throws AcceptsException, ProcessException { | |||
| List<String> currentProcessorChain = Collections.synchronizedList(new ArrayList<>()); | |||
| List<String> currentProcessorChain = new ArrayList<>(); | |||
| for (List<ContextProcessor> processorsSamePriority : contextProcessors.values()) { | |||
| processorsSamePriority.parallelStream().forEach(processor -> { | |||
There was a problem hiding this comment.
Remove .parallelStream() to disable parallel execution.
|
@dhughes-xumak the discussion last week was that for 1.0 release is to leave parallel execution out for now so that we can move forward. If you don't mind, please take this task of restoring non parallel execution. There's seemed to be a lot of changes going on. |
|
Have @jbarrera-xumak or @neozilon review after you restore. Thanks |
dhughes-xumak
force-pushed
the
revert/parallel-CP-execution
branch
from
1a8ef6f
to
0d4a32c
Compare
|
That discussion occurred before the necessary fixes were proposed for the AEM and JahiaDF projects. Parallel execution is disabled with a single change, as explained in my review comment. I've pushed a new commit to this branch to disable it. |
dhughes-xumak
dismissed
their stale review
The rejected changes have been replaced with the correct approach. I still question whether it is necessary to disable this at all.
|
@neozilon / @jbarrera-xumak, please advise when you review the changes to see whether we should disable or keep parallel execution for 1.0 release. I think it's good to keep, but based on the discussion last week, there seemed to be some issues with it.. Although, @dhughes-xumak seemed to mention that something was done to AEM and JahiaDF which resolved the issues. |
|
@jbarrera-xumak @neozilon @dhughes-xumak I guess we can delete this PR then. Right? |
No description provided.