search

thomasp85 / grid

personal devel version of grid
15 stars 4 forks source link

POC on L_points for optimising unit conversion in simpleUnits #16

Open thomasp85 opened 4 years ago

thomasp85 commented 4 years ago

This PR provides a first stab at factoring out the unit conversion of the input positions during grob drawing. The POC has been done on L_points.

Basic approach is to check whether both x and y are simpleUnit vectors. If so, a conversion factor is calculated once and applied to the whole vector. If either of the vectors are not simpleUnit objects it will fall back to the standard element-wise conversion.

The same approach should be applicable for converting widths and heights but it is not implemented yet.

One thing to consider is that a few unit types are dependent on gpar settings, so even in the case of simpleUnit vectors we need to make sure that e.g. line height is constant over the whole vector.

Preliminary testing gives >10x speedup in grid.points() with 1e5 points, which is insane and actually makes grid.points() faster than points()

@pmur002 can you have a look at this POC and maybe identify some wrong assumptions I may have made before I try to apply it to all the grobs

thomasp85 commented 4 years ago

Just to stifle the excitement. The 10x speedup and edge over base graphic is based on benchmarking with the devoid package to factor out device specific rendering time. A lot goes away when actually rendering the output as well. Somehow base graphics gets faster again also, which weirds me out

pmur002 commented 4 years ago

Thanks. I think I follow the idea and I think the code makes sense.

A couple of points:

The test (isSimpleUnit(x) && isSimpleUnit(y)) discards "stringWidth" et al and "grobWidth" et al (which is good), but keeps "char", "lines", and "null". The first one depends on gc->ps and gc->cex, the second one depends on those and gc->lineheight, so, as you point, out there would need to be stricter tests for them. The third one, "null", should probably also be discarded. "sum", "min", and "max" are also kept, but I don't think it is possible to create a "sum" "simpleUnit" (?)

Regarding speed, parity with base graphics would still be very nice :)