Skip to content

fix issue 1672 (inefficient logging in Node.java) #1673

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
Merged
merged 2 commits into from
Nov 24, 2021
Merged

fix issue 1672 (inefficient logging in Node.java) #1673

merged 2 commits into from
Nov 24, 2021

Conversation

@vxel
Copy link
Contributor

@vxel vxel commented Nov 23, 2021

This fixes issue #1672

@stephengold stephengold linked an issue Nov 23, 2021 that may be closed by this pull request
Logging statement should be guarded #1672
Closed
@stephengold stephengold added this to the v3.5.0 milestone Nov 23, 2021
@stephengold
Copy link
Member

Looks good to me! Unless there's substantial review, I will integrated this in about 24 hours.

@stephengold stephengold changed the title #1672 Logging statements should be guarded fix issue 672 (Logging statements should be guarded) Nov 23, 2021
@stephengold stephengold changed the title fix issue 672 (Logging statements should be guarded) fix issue1 672 (Logging statements should be guarded) Nov 23, 2021
@stephengold stephengold changed the title fix issue1 672 (Logging statements should be guarded) fix issue 1672 (Logging statements should be guarded) Nov 23, 2021
@pspeed42
Copy link
Contributor

I kind of wish we'd just changed the "this.toString()" into just "this" instead. That's a garbage anti-idiom that would have prevented the whole issue if done properly in the first place.

@stephengold
Copy link
Member

I like pspeed's solution better than adding guard code. @vxel would you be willing to implement that solution instead?

@vxel
Copy link
Contributor Author

vxel commented Nov 24, 2021

Agree with that too. PR updated.

PS: sorry for the force-push I merged wrong commits...

@stephengold
Copy link
Member

Force pushes are okay in this case because "fix/1672" is your own private branch. But unnecessary because I'm going to squash the history when I integrate this PR.

@stephengold stephengold changed the title fix issue 1672 (Logging statements should be guarded) fix issue 1672 (inefficient logging in Node.java) Nov 24, 2021
@stephengold stephengold merged commit a12cd0d into jMonkeyEngine:master Nov 24, 2021
@vxel vxel deleted the fix/1672 branch November 24, 2021 20:06
@stephengold
Copy link
Member

Thanks for your contribution, @vxel !

vxel added a commit to vxel/jmonkeyengine that referenced this pull request Nov 25, 2021
@vxel
fix issue 1672 (inefficient logging in Node.java) (jMonkeyEngine#1673)
dc4ab30 
* jMonkeyEngine#1672 Logging statements should be guarded

* jMonkeyEngine#1672 Fix performance penalty of logger without guard

Co-authored-by: Cédric de Launois <>
@stephengold stephengold modified the milestones: v3.5.0, v3.4.1 Dec 3, 2021
stephengold pushed a commit that referenced this pull request Dec 4, 2021
@vxel @stephengold
fix issue 1672 (inefficient logging in Node.java) (#1673)
662c8fe 
* #1672 Logging statements should be guarded

* #1672 Fix performance penalty of logger without guard

Co-authored-by: Cédric de Launois <>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

v3.4.1

Development

Successfully merging this pull request may close these issues.

Logging statement should be guarded

3 participants

@vxel @stephengold @pspeed42