search

NOAA-EMC / NCEPLIBS-bufr

The NCEPLIBS-bufr library contains routines and utilites for working with the WMO BUFR format.
Other
42 stars 19 forks source link

Jack Woollen's fix to optimize upb8.f routine #534

Closed jbathegit closed 9 months ago

jbathegit commented 9 months ago

Courtesy of @jack-woollen, and per the discussions in GSI/589 and GSI/642, this patch to subroutine upb8 optimizes performance for the majority of values which are encoded in fields of 32 bits or less.

jbathegit commented 9 months ago

BTW, I saw in the discussions how there's more to come w.r.t. improving the GSI timings, but in the meantime I don't see any reason not to go ahead and get this into the library baseline, since it's certainly a useful update :-)

jack-woollen commented 9 months ago

Here's an interesting fact I stumbled on. As you know version 11.7.1 has the same ability to pack up numbers in 64 bits as v12.0.0. When I test 11.7.0, 11.7.1, and 12.0.0 against each other, both 11.7.1 and 12.0.0 show the same time increase against 11.7.0. Which means the imb8 blocks have nothing to do with the slowdown. It narrows the focus to what's slowing down 11.7.1, which is a much smaller change (I think) than 12.0.0. I can approve this if you want but there may be another change or two worth making sometime soon.

jbathegit commented 9 months ago

Yeah, that makes perfect sense Jack, given that the first issue you found was this one in upb8, and version 11.7.1 was indeed the first library release which included the capability to encode values larger than 32 bits (as noted in the Release Notes ;-)

So that was a good catch, and yes that should considerably narrow down the scope of the search!

In the meantime, yes please approve this so we can go ahead and at least get this upb8 upgrade into the code baseline, but I completely understand that there's likely more to come and which can always be added via one or more additional PRs at a later date.