Improved concatenation way in PropertiesStorage.java #7486

nikam14 · 2023-05-02T10:00:45Z

Description

This PR... changed the '+' operator for concatenation to .concat method for concatenation because '+' operator takes more time then .concat method.
[Optimisation].

Types of changes

Breaking change (fix or feature that would cause existing functionality to change)
New feature (non-breaking change which adds functionality)
Bug fix (non-breaking change which fixes an issue)
Enhancement (improves an existing feature and functionality)
Cleanup (Code refactoring and cleanup, that may add test cases)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Major
Minor

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

yadvr · 2023-05-08T06:42:33Z

agent/src/main/java/com/cloud/agent/dao/impl/PropertiesStorage.java

            try {
                if (!file.createNewFile()) {
-                    s_logger.error("Unable to create _file: " + file.getAbsolutePath());
+                    s_logger.error("Unable to create _file: ".concat(file.getAbsolutePath()));


the preferred way would be to use String.format, the concatenation using + isn't any different from concat.

thank you for your suggestion , I will change it .

DaanHoogland

clgtm, but why these particular instances of the + operator, @nikam14 ? Are these particular detrimental to performance in your system as opposed to the rest of the logging happening?

codecov · 2023-05-08T07:44:47Z

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (8b5bfb1) 12.72% compared to head (76666bb) 28.54%.
Report is 489 commits behind head on main.

Files	Patch %	Lines
...va/com/cloud/agent/dao/impl/PropertiesStorage.java	0.00%	4 Missing ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##               main    #7486       +/-   ##
=============================================
+ Coverage     12.72%   28.54%   +15.82%     
- Complexity     8706    30290    +21584     
=============================================
  Files          2727     5192     +2465     
  Lines        256526   366145   +109619     
  Branches      39989    53530    +13541     
=============================================
+ Hits          32636   104523    +71887     
- Misses       219738   247199    +27461     
- Partials       4152    14423    +10271

Flag	Coverage Δ
simulator-marvin-tests	`24.30% <0.00%> (?)`
uitests	`4.49% <ø> (?)`
unit-tests	`14.81% <ø> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

yadvr · 2023-05-23T05:45:50Z

@blueorangutan package

blueorangutan · 2023-05-23T05:46:03Z

@rohityadavcloud a [SF] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

blueorangutan · 2023-05-23T08:20:34Z

Packaging result: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 6115

soreana

LGTM

soreana · 2023-09-30T21:09:02Z

@blueorangutan package

blueorangutan · 2023-09-30T21:10:03Z

@soreana a [SF] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

soreana · 2023-10-01T16:11:40Z

@blueorangutan package

blueorangutan · 2023-10-01T16:18:03Z

@soreana a [SF] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

soreana · 2023-10-01T20:32:39Z

@blueorangutan package

blueorangutan · 2023-10-01T20:34:03Z

@soreana a [SF] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

blueorangutan · 2023-10-01T21:28:30Z

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 7183

agent/src/main/java/com/cloud/agent/dao/impl/PropertiesStorage.java

soreana · 2023-10-02T17:43:58Z

@nikam14 Can you please address @GutoVeronezi 's comments?

shwstppr · 2023-10-05T08:20:14Z

@nikam14 can you please address outstanding review comments?

Co-authored-by: Daniel Augusto Veronezi Salvador <38945620+GutoVeronezi@users.noreply.github.com>

DaanHoogland · 2023-10-31T14:58:04Z

@nikam14 @GutoVeronezi , I've applied the rather trivial changes to move this forward, please publicaly shame me here or on email if that was just to brutal of me.

DaanHoogland · 2023-10-31T14:58:18Z

@blueorangutan package

blueorangutan · 2023-10-31T15:00:04Z

@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

DaanHoogland

small improvement allowing operators to supress stacktraces

agent/src/main/java/com/cloud/agent/dao/impl/PropertiesStorage.java

blueorangutan · 2023-10-31T15:59:19Z

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 7584

….java

agent/src/main/java/com/cloud/agent/dao/impl/PropertiesStorage.java

….java

DaanHoogland · 2023-11-15T15:08:46Z

formatting log messages only, should be good if compiles

Co-authored-by: dahn <daan.hoogland@gmail.com> Co-authored-by: Daniel Augusto Veronezi Salvador <38945620+GutoVeronezi@users.noreply.github.com>

[MINOR][Enhancement] Update PropertiesStorage.java

f70061b

boring-cyborg bot added the component:agent label May 2, 2023

yadvr reviewed May 8, 2023

View reviewed changes

DaanHoogland approved these changes May 8, 2023

View reviewed changes

changed to String.format

5c5f698

yadvr added this to the 4.19.0.0 milestone May 23, 2023

yadvr approved these changes May 23, 2023

View reviewed changes

soreana approved these changes Sep 30, 2023

View reviewed changes

soreana closed this Oct 1, 2023

soreana reopened this Oct 1, 2023

GutoVeronezi reviewed Oct 2, 2023

View reviewed changes

agent/src/main/java/com/cloud/agent/dao/impl/PropertiesStorage.java Outdated Show resolved Hide resolved

agent/src/main/java/com/cloud/agent/dao/impl/PropertiesStorage.java Outdated Show resolved Hide resolved

DaanHoogland added type:enhancement Severity:Minor labels Oct 3, 2023

DaanHoogland changed the title ~~[MINOR][Enhancement] improved concatenation way in PropertiesStorage.java~~ Improved concatenation way in PropertiesStorage.java Oct 3, 2023

Apply (trivial) suggestions from code review

0434fb2

Co-authored-by: Daniel Augusto Veronezi Salvador <38945620+GutoVeronezi@users.noreply.github.com>

DaanHoogland approved these changes Oct 31, 2023

View reviewed changes

agent/src/main/java/com/cloud/agent/dao/impl/PropertiesStorage.java Outdated Show resolved Hide resolved

Update agent/src/main/java/com/cloud/agent/dao/impl/PropertiesStorage…

4186baf

….java

DaanHoogland reviewed Nov 15, 2023

View reviewed changes

agent/src/main/java/com/cloud/agent/dao/impl/PropertiesStorage.java Outdated Show resolved Hide resolved

Update agent/src/main/java/com/cloud/agent/dao/impl/PropertiesStorage…

76666bb

….java

DaanHoogland closed this Nov 15, 2023

DaanHoogland reopened this Nov 15, 2023

DaanHoogland assigned DaanHoogland and nikam14 Nov 15, 2023

soreana approved these changes Nov 16, 2023

View reviewed changes

DaanHoogland merged commit cc45bff into apache:main Nov 16, 2023

DaanHoogland unassigned DaanHoogland and nikam14 Nov 16, 2023

Improved concatenation way in PropertiesStorage.java #7486

Improved concatenation way in PropertiesStorage.java #7486

Uh oh!

Conversation

nikam14 commented May 2, 2023

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

Uh oh!

yadvr May 8, 2023

Choose a reason for hiding this comment

Uh oh!

nikam14 May 9, 2023

Choose a reason for hiding this comment

Uh oh!

DaanHoogland left a comment

Choose a reason for hiding this comment

Uh oh!

codecov bot commented May 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

yadvr commented May 23, 2023

Uh oh!

blueorangutan commented May 23, 2023

Uh oh!

blueorangutan commented May 23, 2023

Uh oh!

soreana left a comment

Choose a reason for hiding this comment

Uh oh!

soreana commented Sep 30, 2023

Uh oh!

blueorangutan commented Sep 30, 2023

Uh oh!

soreana commented Oct 1, 2023

Uh oh!

blueorangutan commented Oct 1, 2023

Uh oh!

soreana commented Oct 1, 2023

Uh oh!

blueorangutan commented Oct 1, 2023

Uh oh!

blueorangutan commented Oct 1, 2023

Uh oh!

Uh oh!

Uh oh!

soreana commented Oct 2, 2023

Uh oh!

shwstppr commented Oct 5, 2023

Uh oh!

DaanHoogland commented Oct 31, 2023

Uh oh!

DaanHoogland commented Oct 31, 2023

Uh oh!

blueorangutan commented Oct 31, 2023

Uh oh!

DaanHoogland left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

blueorangutan commented Oct 31, 2023

Uh oh!

Uh oh!

DaanHoogland commented Nov 15, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

codecov bot commented May 8, 2023 •

edited

Loading