[RF][HS3] JSONIO init order bugfixes

[cburgard]

This Pull request:

When instantiating an Exporter like this:

STATIC_EXECUTE([]() {
  using namespace RooFit::JSONIO;
  
  registerImporter<MYCLASSFactory>("key", false);
  registerExporter<MYCLASSStreamer>(MYCLASS::Class(), false);
 });

it is possible that this will segfault, as MYCLASS might not be sufficiently loaded yet to permit MYCLASS::Class() to be called.

In order to circumvent this problem, a new method has been added:

With registerExporter<MYCLASSStreamer>("MYCLASS", false);, it is now possible to add an exporter in a "delayed" fashion, where it will only be actually added to the exporter list when to RooJSONFactoryWSTool is invoked - before that, it will linger in a temprorary "Loader" helper object.

Changes or fixes:

Checklist:

• tested changes locally
• updated the docs (if necessary)

This PR fixes #

[guitargeek]

Question: is the TClass version of registerExporter still useful for public use at this point?

[github-actions]

Test Results

    21 files      21 suites   3d 4h 55m 1s ⏱️
 3 796 tests  3 796 ✅ 0 💤 0 ❌
71 772 runs  71 772 ✅ 0 💤 0 ❌

Results for commit f57672f.

♻️ This comment has been updated with latest results.

[cburgard]

Question: is the TClass version of registerExporter still useful for public use at this point?

I think it would probably be safer to make them internal, such that people only use the string variant.

As a further improvement, I think it might be beneficial to move the call to setupKeys() to the same point in the code, since the early call to TClass can also be a problem there.

[silverweed]

Why are two maps needed? Isn't it an error (at least logically speaking) to register the same key twice with different priorities?

Also top and bottom are not very clear names imho, I would use something a little more descriptive.

[cburgard]

I'm happy with renaming them in any way you liked, I just found it more elegant to keep two maps separately (one for the ones to be added at the top, and one for the ones to be added at the bottom of the priority hierarchy) rather than introducing a custom type just to be able to annotate the exporter with a boolean. But if you feel like another way is more elegant, feel free to change it up!

[guitargeek]

Thanks for the comments, very true! I'm with Carsten at the CMS hackathon and we're trying to find a better solution.

[pcanal]

The following will have the same net effect but (slightly) reduce the number of conditionals upon calling the function again.

Suggested change

	static bool isAlreadySetup = false;
	if (isAlreadySetup) {
	return;
	}
	isAlreadySetup = true;
	std::stringstream factoryexpressions;
	factoryexpressions << RooFitHS3_wsfactoryexpressions;
	loadFactoryExpressions(factoryexpressions);
	static bool isAlreadySetup = []() {
	std::stringstream factoryexpressions;
	factoryexpressions << RooFitHS3_wsfactoryexpressions;
	loadFactoryExpressions(factoryexpressions);
	return true;
	}();

(and/or a version with std::call_once).

[pcanal]

Do we support the flow

• register exporter
• use them
• register some more exporter
• use the old and new ones
  ?
  If we do consider using

Suggested change

	return exportersImpl();
	exportersToAdd().clear();
	return exportersImpl();

to avoid work duplication (and/or redundant adds)
If we do not, consider adding an explicit failure more so that the user is not surprised that the later added ones are ignored or what not).