`shiny_attributes` performance improvments about eml HOT 8 CLOSED

Thanks for the report, @jeanetteclark. We should definitely invest some time getting this working again. Your idea about line 190 being a culprit seems pretty reasonable after a quick glance. It does seem to do work that is identical on every invocation.

It would probably help me to get an MRE for this so I can poke around. I'll try that now and get back to you. Does this happen with attribute lists of any size or just larger ones? I'd also like to get at the bottom of why this is happening now and not before.

from eml.

@amoeba it actually happens for me even if I don't pass any arguments and it just spins up an empty table. Typically, it never loads, or it takes several minutes, which leads me to believe I'm probably missing something. I got to that particular line by running profvis on get_unit_id

from eml.

Typically, it never loads, or it takes several minutes

Very interesting. It is slow to load but I get it to load in about 8s on both my local machine and datateam. That's still too slow for my test but I'm curious why it's so much slower for you. Can you dump a devtools::session_info() in here?

from eml.

After cleaning up my session a bit I am also down to 8s or so to load a blank table, and somewhat reasonable (but still a little slow) times for tables with moderate number of attributes

from eml.

@jeanetteclark sent me a devtools::session_info() out of band of this conversation though it didn't give me any pointers.

I was able to reproduce the performance @jeanetteclark was seeing and saw the same culprit. I opted to use an Environment to cache the units::valid_udunits_prefixes call and test that the startup time and editing slowness went away.

See #333. @jeanetteclark would you mind giving that a whirl just to confirm it works equally well for you?

from eml.

Cool @amoeba! I thought about caching, but wouldn't have thought to use a global var for it. I just tested and it works much better. I can have a look at those tests and see if I can fix them this afternoon

from eml.

Sounds good, thanks @jeanetteclark.

from eml.

Closing since this change was merged and we're cutting a release.

from eml.