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.

Sign up for GitHub

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

Jump to bottom

Use Deque/ArrayDeque instead of Stack in RequestProcessor class #6267

Merged
merged 1 commit into from
Aug 6, 2023
Merged

Use Deque/ArrayDeque instead of Stack in RequestProcessor class #6267

merged 1 commit into from
Aug 6, 2023

Conversation

pepness
Copy link
Member

@pepness pepness commented Jul 28, 2023

NetBeans Notes:

  • Use Deque/ArrayDeque instead of Stack, the Deque interface should be used in preference to the legacy Stack class, it will give better performance and it is already used in synchronized blocks so there is no need for the thread-safe Stack/Vector class.
  • Increase default RequestProcessor throughput from 50 to 55.

Testing:

  • Full build done
  • Verify successful execution of libraries and licenses Ant test
  • Verify successful execution of unit tests for module platform.openide.util
  • Verify successful execution of sigtests
  • Verify successful execution of commit-validation
  • Worked normally with various projects.

@pepness
- Use Deque/ArrayDeque instead of Stack, the Deque interface should be
cc63dbd 
used in preference to the legacy Stack class, it will give better
performance and it is already used in synchronized blocks so there is no
need for the thread-safe Stack/Vector class.
- Increase default RequestProcessor throughput from 50 to 55.
@pepness pepness added the Platform [ci] enable platform tests (platform/*) label Jul 28, 2023
@pepness pepness added this to the NB20 milestone Jul 28, 2023
@pepness pepness self-assigned this Jul 28, 2023
@pepness pepness added ci:all-tests [ci] enable all tests and removed Platform [ci] enable platform tests (platform/*) labels Jul 29, 2023
sdedic
sdedic approved these changes Jul 31, 2023
View reviewed changes
Copy link
Member

@sdedic sdedic left a comment

Choose a reason for hiding this comment

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

Seems OK, I didn't find any semantic change (except the incrased throughput). If the throughput limit has to be increased, what about a configurable (system property) value with 55 as the default ?

@pepness
Copy link
Member Author

pepness commented Aug 6, 2023

I agree, that way it will be easily for someone to experiment with lower or higher throughput. Will work on that too.

@pepness pepness merged commit 7d16bfe into apache:master Aug 6, 2023
34 checks passed
@mbien mbien added the Platform [ci] enable platform tests (platform/*) label Aug 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sdedic sdedic sdedic approved these changes

Assignees

@pepness pepness

Labels
ci:all-tests [ci] enable all tests Platform [ci] enable platform tests (platform/*)
Projects
None yet
Milestone
NB20
Development

Successfully merging this pull request may close these issues.
3 participants
@pepness @sdedic @mbien