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

How to improve tps in multithreading #340

Closed
shiyong-1224 opened this issue Jun 27, 2024 · 5 comments · Fixed by #352 or #353
Closed

How to improve tps in multithreading #340

shiyong-1224 opened this issue Jun 27, 2024 · 5 comments · Fixed by #352 or #353
Assignees
Milestone

Comments

@shiyong-1224
Copy link

I am using a ThreadPoolExecutor to execute a pdf rendering task (flying-saucer-pdf:9.1.22 Java 11), here is the main code of each task:

ITextRenderer renderer = new ITextRenderer();
renderer.setDocumentFromString(html);
renderer.layout();
renderer.createPDF(outputStream);

It performs best when I set corePoolSize = 2
As I increase the corePoolSize, the time consuming of 'layout' and 'createPDF' method increases and the tps decreases.
Even if I increased the CPU from 2 cores to 8 cores, it didn't help.
so I wonder if there is any resource contention or lock between threads, and how can I improve the tps of single machine

@asolntsev
Copy link
Contributor

@shiyong-1224 Sorry, but I cannot reproduce this issue. When I generate PDFs in parallel threads, they work fast. More threads -> faster generation.
Can you create a sample project for reproducing the problem?

@shiyong-1224
Copy link
Author

@asolntsev Thanks for your Reply,here is a sample project https://github.com/shiyong-1224/test-pdf.git

@asolntsev
Copy link
Contributor

@shiyong-1224 Thank you for sharing the project, but ... again, how I could easily reproduce the problem?
What script should I run? I assume (from README) that you use JMeter, but you haven't committed any JMeter scripts.

  1. I just executed your code in plain JUnit tests (not in Spring app), and I still don't see a performance degradation in parallel execution.
  2. however, when I run your project (in Spring app), then I see the performance degradation.

Is it possible that the problem is caused by Spring/Tomcat/servlets/etc... ?

Anyway, I've found few candidates for performance improvement in FlyingSaucer. I will release these improvements soon, but not sure it entirely fixes your problem.

@asolntsev asolntsev added this to the 9.8.1 milestone Jun 30, 2024
@asolntsev asolntsev self-assigned this Jun 30, 2024
@shiyong-1224
Copy link
Author

@asolntsev Sorry I didn't describe it clearly.
That is how to reproduce the problem:

  1. run the sample project as Spring web app
  2. curl http://localhost:15000/pdf
  3. then you can I see the time consumed of each execution of PdfUtils.renderPdf() method

I changed corePoolSize from 2 to 8 to 16, and use Jmeter(uploaded a script) to make full use of threads, Then I saw the time consumed increasing by num of threads.
I found these 2 methods layout() & createPDF consumed more time.
I've read some source codes of these 2 methods, have no idea about the appearance.
As for Spring/Tomcat/servlets or any other frameworks, I don't known how they affect the performance of layout() & createPDF
Anyway, in my real production environment, I can increase the tps by adding more instances of the pdf service cluster.

asolntsev added a commit that referenced this issue Jun 30, 2024
My profiler shows that some remarkable % of CPU is spent on generating this string: (" " + c + " "). Now we check for CSS class without generating the new string. The code is longer, but faster.
asolntsev added a commit that referenced this issue Jun 30, 2024
Now we check the "lang" attribute without calling `String.split` method which generates new strings.
asolntsev added a commit that referenced this issue Jun 30, 2024
in some places, I replaced them with `synchronizedMap()` or `ConcurrentHashMap`.
Not 100% sure nothing gets broken. :)
But at least all tests are still green.
asolntsev added a commit that referenced this issue Jul 19, 2024
I don't know what was the initial idea, and was it intended to pass any other parameters, but currently only `org.w3c.dom.Node` is passed there.
asolntsev added a commit that referenced this issue Jul 19, 2024
My profiler shows that `nsh.getClass((Element) e)` consumes quite a remarkable percentage of total CPU time.
So I hope caching these value could improve performance (though, my profiler doesn't show it :) ).
@asolntsev asolntsev linked a pull request Jul 19, 2024 that will close this issue
asolntsev added a commit that referenced this issue Jul 19, 2024
My profiler shows that some remarkable % of CPU is spent on generating this string: (" " + c + " "). Now we check for CSS class without generating the new string. The code is longer, but faster.
asolntsev added a commit that referenced this issue Jul 19, 2024
Now we check the "lang" attribute without calling `String.split` method which generates new strings.
asolntsev added a commit that referenced this issue Jul 19, 2024
in some places, I replaced them with `synchronizedMap()` or `ConcurrentHashMap`.
Not 100% sure nothing gets broken. :)
But at least all tests are still green.
asolntsev added a commit that referenced this issue Jul 19, 2024
I don't know what was the initial idea, and was it intended to pass any other parameters, but currently only `org.w3c.dom.Node` is passed there.
asolntsev added a commit that referenced this issue Jul 19, 2024
My profiler shows that `nsh.getClass((Element) e)` consumes quite a remarkable percentage of total CPU time.
So I hope caching these value could improve performance (though, my profiler doesn't show it :) ).
asolntsev added a commit that referenced this issue Jul 19, 2024
asolntsev added a commit that referenced this issue Jul 19, 2024
asolntsev added a commit that referenced this issue Jul 19, 2024
…in indexOf()calls"

A quick-fix is suggested to replace such string literals with equivalent character literals, gaining some performance enhancement.
asolntsev added a commit that referenced this issue Jul 19, 2024
@asolntsev asolntsev linked a pull request Jul 19, 2024 that will close this issue
asolntsev added a commit that referenced this issue Jul 19, 2024
asolntsev added a commit that referenced this issue Jul 19, 2024
asolntsev added a commit that referenced this issue Jul 19, 2024
…in indexOf()calls"

A quick-fix is suggested to replace such string literals with equivalent character literals, gaining some performance enhancement.
asolntsev added a commit that referenced this issue Jul 19, 2024
@asolntsev
Copy link
Contributor

@shiyong-1224 Hi. Can you check if the problem was fixed in FlyingSaucer 9.9.0?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants