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

Use exception catching, possibly unnecessary performance overhead #387

Closed
fengyizhu opened this issue Nov 14, 2022 · 7 comments
Closed

Use exception catching, possibly unnecessary performance overhead #387

fengyizhu opened this issue Nov 14, 2022 · 7 comments
Assignees
Milestone

Comments

@fengyizhu
Copy link

tech.units.indriya.function.DefaultNumberSystem#narrow

        if(number instanceof BigDecimal) {
            
            final BigDecimal decimal = ((BigDecimal) number);
            try {
                BigInteger integer = decimal.toBigIntegerExact(); 
                return narrow(integer);
            } catch (ArithmeticException e) {
                return number; // cannot narrow to integer
            }
        }

why not just return BigDecimal?

@fengyizhu
Copy link
Author

image

@keilw
Copy link
Member

keilw commented Nov 14, 2022

@fengyizhu What does this graph show, and is there a test suite you can share to replicate?

@andi-huber What was the BigInteger extraction done there? I understand isIntegerOnly() check simply does a Java type check, but why not apply isInteger() or try reuse the if(decimal.scale()<=0) part instead of trying the exception first?

@fengyizhu
Copy link
Author

#388

@fengyizhu
Copy link
Author

@keilw Please See #388

@andi-huber
Copy link
Member

Indeed, producing stacktraces just to do a check is costly. I merged #388

see also https://stackoverflow.com/questions/1078953/check-if-bigdecimal-is-an-integer-in-java

@fengyizhu
Copy link
Author

@andi-huber @keilw Is there a plan and time to release a revised version?

@keilw keilw added this to the 2.1.4 milestone Nov 15, 2022
@keilw
Copy link
Member

keilw commented Nov 15, 2022

The scope is here: https://github.com/unitsofmeasurement/indriya/milestone/10 but there's not a concrete time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: Done
Development

No branches or pull requests

3 participants