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

Performance #298

Closed
parnoldx opened this issue Aug 6, 2020 · 9 comments
Closed

Performance #298

parnoldx opened this issue Aug 6, 2020 · 9 comments

Comments

@parnoldx
Copy link

parnoldx commented Aug 6, 2020

Hi, not sure if this is a issue for you but it seems that the 2.0 implementation is somehow not really performant. For the simple addition test below, I see these numbers in ms

primitives 8
bigdecimal 586
quantities 11762
                int count = (int) 10E5;
		for (int i = 0; i < 1000; i++) {
			double c = 2 + 3.4 + 5.5;
		}
		Stopwatch time = Stopwatch.createStarted();
		for (int i = 0; i < count; i++) {
			double a = 5;
			double b = 5.2;
			double c = a + b;
			doSomething(c);
		}
		System.out.println("primitives "+time.elapsed(TimeUnit.MILLISECONDS));
		time = Stopwatch.createStarted();
		for (int i = 0; i < count; i++) {
			BigDecimal a = new BigDecimal(5);
			BigDecimal b = new BigDecimal(5.2);
			BigDecimal c = a.add(b);
			doSomething(c);
		}
		System.out.println("bigdecimal "+time.elapsed(TimeUnit.MILLISECONDS));
		time = Stopwatch.createStarted();
		Unit<?> parse = SimpleUnitFormat.getInstance().parse("mm");
		for (int i = 0; i < count; i++) {
			Quantity a = Quantities.getQuantity(5, parse);
			Quantity b = Quantities.getQuantity(5.2, parse);
			Quantity add = a.add(b);
			doSomething(add.getValue());
		}
		System.out.println("quantities "+time.elapsed(TimeUnit.MILLISECONDS));
@keilw
Copy link
Member

keilw commented Aug 6, 2020

Thanks for mentioning that. We had some benchmarks very early on by @desruisseaux but I believe that is no longer there or vanished with the JSR 275 codebase.
If you say 2.0 is not performant, do you see any different results trying this with 1.0?

The better performance of primitive types is not new, but they are primitive and you would not want to sacrifice the data integrity or more as the Mars Climate Orbiter showed (although they may not use Java directly on the spacecraft;-)

The performance penalty is known, although your example is not the best comparison, since you combine parsing/formatting with the most primitive numerical operations even using BigDecimal.
If you wanted to get that somewhat to match, why not use DecimalFormat or similar with BigDecimal and another NumberFormat with the primitive types?

However, you will always need more space to deal with quantities than primitive numbers.
There is a compromise because UnitConverter works with numbers (also primitives like double) and you don't need to instantiate a Quantity, so it works faster for mass operations like the conversion of thousands of data points per minute or so, that should work with UnitConverter and low level primitive numbers while it could be challenging if you create a Quantity instance for each of them.

HTH, I don't think the snippet works as a real benchmark, but you are more than welcome to contribute to that, if you can.

@keilw keilw closed this as completed Aug 6, 2020
@parnoldx
Copy link
Author

parnoldx commented Aug 6, 2020

The result for 1.0 is

primitives 7
bigdecimal 472
quantities 340

@parnoldx
Copy link
Author

parnoldx commented Aug 6, 2020

Not sure what you mean with "real benchmark", sure this is artificially to highlight the problem?! But the use case is to replace all numbers with quantities to assure correct calculation according to assigned units in a system where you do a lot of math and data manipulation.
I "tweaked" 1.0 a bit for my use case and came to a penalty of ~ 10 so for the example above I get ~80ms for the quantities part. Which is ok for not to sacrifice data integrity but not this penalty in 2.0. IMO

@keilw
Copy link
Member

keilw commented Aug 6, 2020

Did you use Apache stopwatch or what is the stopwatch?
I mean a JMH Benchmark, it is part of the JDK now for a while (after Java 11)

The parsing should not be used in one case only, instantiating a new Quantity with the Units constant seems more appropriate in this case. Do you have some code snippets including Apache Stopwatch we could try out?

I created #299 for a true benchmark.

@andi-huber
Copy link
Member

Indriya 2.0.x does calculation with higher precision than 1.x. Also note, that you can speed up calculations above by being more careful when picking operand values for comparison. (might be 2 orders of magnitude faster) ...

// instead of
BigDecimal b = new BigDecimal(5.2); 
// use 
BigDecimal b = new BigDecimal("5.2");

// instead of
Quantities.getQuantity(5.2, Units.METRE); 
// use 
Quantities.getQuantity(RationalNumber.of(52, 10), Units.METRE);

@keilw
Copy link
Member

keilw commented Aug 6, 2020

Thanks for the input @andi-huber, could you maybe with help by @parnold-x if he wishes to help and others like @desruisseaux if he has time think about a benchmark along the lines of https://github.com/JavaMoney/javamoneyjmh? I forked that based on something a contributor did earlier. We don't have to use Gradle, since Maven is currently used everywhere else, but the idea would be to create some benchmarks that show also how using RationalNumber instead of BigDecimal can impact the performance. This should not go here, but either under https://github.com/unitsofmeasurement/uom-tools where checkstyle rules are also parked among other tools, or in a separate repo.

@andi-huber
Copy link
Member

I've created a gist, not a best practice benchmark, but gives some idea on what to look for. Testing 3 aspects of quantity calculus:

  1. adding 2 quantities
  2. multiplying a quantity with a number (scalar multiplication)
  3. multiplying 2 quantities

https://gist.github.com/andi-huber/92ab34634560f22647adab584041144c

Showing

-- ADD
bigdecimal 21 ms
quantities 361 ms
-- MUL
bigdecimal 14 ms
quantities 328 ms
-- SCALAR MUL
bigdecimal 12 ms
quantities 273 ms

Roughly speaking, Indriya is 20 times slower than when using BigDecimal directly. However, this sound pretty reasonable to me.

@keilw
Copy link
Member

keilw commented Aug 6, 2020

I think if the JUnit test itself does not take too long, we could add it to the JUnit tests, if it is very slow, then it should be excluded like some LocalUnitFormat tests because they occasionally cause resource overflows during the whole build (something we might like to explore, but LocalUnitFormat is also not a core feature and mostly for GUI rendering, not unit exchange between systems)

@keilw
Copy link
Member

keilw commented Aug 6, 2020

I added it under https://github.com/unitsofmeasurement/indriya/blob/master/src/test/java/tech/units/indriya/IndriyaPerformanceTest.java, with a few tweaks like log outputs instead of System.out. So it can be run with a custom logging properties while by default it won't pollute the regular builds.

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

No branches or pull requests

3 participants