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

Parallelism Pulse entities in parallel using divide and conquer #1008

Open
wants to merge 4 commits into
base: dev
Choose a base branch
from
Open

Parallelism Pulse entities in parallel using divide and conquer #1008

wants to merge 4 commits into from

Conversation

Hagvan
Copy link

@Hagvan Hagvan commented Jan 23, 2019

Utilizes CPU resources better.
I tested this using artificial load inside GlowEntity.pulse() method - each pulse every entity generates 10000 random integers.
Here are my benchmarks (CPU: Ryzen 2600X):
A message is shown every 120 ticks inside each world thread.

Single threaded
single thread

ForkJoinPool
forkjoinpool

Possible concerns:
I know in the future AI for mobs will be added and right now I don't know for sure if it will affect it.
What I did is just speed up iteration inside the GlowWorld.pulse() method.

to pulse entities recursively in parallel (divide and conquer).
entities recursively in parallel (divide and conquer).
Copy link
Member

@mastercoms mastercoms left a comment

Choose a reason for hiding this comment

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

Will leave full review later when I have time, but please fix these style issues.

invokeAll(new ForkPulse(entities, left, mid),
new ForkPulse(entities, mid + 1, right));
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Add a new line at the end of file.

Copy link
Author

Choose a reason for hiding this comment

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

I added a new line at the end but it still refuses to build.

invokeAll(new ForkPulsePlayers(players, left, mid),
new ForkPulsePlayers(players, mid + 1, right));
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Add a new line at the end of file.

Copy link
Author

Choose a reason for hiding this comment

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

I added a new line at the end but it still refuses to build.

@mastercoms
Copy link
Member

Would it be possible to use the forkJoinPool branch and make these as GlowTasks?

@BEllis
Copy link

BEllis commented Jan 28, 2019

Just re-iterating (excuse the pun) what I said in discord,

Would the Spliterator.trySplit not be a bit more concise?
i.e. (taken from https://docs.oracle.com/javase/8/docs/api/java/util/Spliterator.html)

static <T> void parEach(TaggedArray<T> a, Consumer<T> action) {
   Spliterator<T> s = a.spliterator();
   long targetBatchSize = s.estimateSize() / (ForkJoinPool.getCommonPoolParallelism() * 8);
   new ParEach(null, s, action, targetBatchSize).invoke();
 }

 static class ParEach<T> extends CountedCompleter<Void> {
   final Spliterator<T> spliterator;
   final Consumer<T> action;
   final long targetBatchSize;

   ParEach(ParEach<T> parent, Spliterator<T> spliterator,
           Consumer<T> action, long targetBatchSize) {
     super(parent);
     this.spliterator = spliterator; this.action = action;
     this.targetBatchSize = targetBatchSize;
   }

   public void compute() {
     Spliterator<T> sub;
     while (spliterator.estimateSize() > targetBatchSize &&
            (sub = spliterator.trySplit()) != null) {
       addToPendingCount(1);
       new ParEach<>(this, sub, action, targetBatchSize).fork();
     }
     spliterator.forEachRemaining(action);
     propagateCompletion();
   }
 }

@mastercoms
Copy link
Member

Thinking about it more, I would really like to have real world situations be tested on this, because of cache locality, world access, and more that would be important to actual entity simulation and AI.

Would you be able to make a more valid test scenario for this PR rather than just generating random numbers?

@Hagvan
Copy link
Author

Hagvan commented Apr 7, 2019

Sorry I was away for a while. I'll try to use actual AI if it is ready at this point.

@mastercoms
Copy link
Member

It isn't ready, but we can hold this PR until then.

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.

3 participants