Timers missing from detailed view in capture

[ZenoArrows]

Not sure what's going on, but after doing a capture on a very slow system the detailed view is empty even though the timer view shows several timers.

MicroProfile Capture.zip

[jonasmr]

Hi.
Its the timer type thats wrong in the html:
Your data looks like this:

S.tt_8_1 = [-2119704132,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0];

This is what it should look like:

S.tt_9_13 = [1,0,1,0,1,0,1,0,1,0,1,0,1,0,1,0,1,0,1,0,1,0,1,0,1,0];

Can you get me a way to reproduce it? A sample program?

[ZenoArrows]

This only happens on the PS Vita as I can't reproduce it on Windows, so I'm not sure how useful a sample program would be. I did have to replace posix_memalign with memalign to get it to compile though, so that could be related.

Also I was using MICROPROFILE_ENTERI and MICROPROFILE_LEAVE rather than the scoped instrumentation.

[ZenoArrows]

I tried the zeux fork and that one does have a working timeline: https://github.com/zeux/microprofile

[ZenoArrows]

When using MICROPROFILE_SCOPEI the timer type is now correct, but there's still nothing on the timeline: MicroProfile Capture.zip

I suspect that Thread IDs are not being returned correctly: S.ThreadIds = [-1,0,0,0,]; Perhaps it can't figure out which bar belongs to which thread? I do wonder how it's even possible for it to write a zero though, given the code that's supposed to write that out:

	MicroProfilePrintf(CB, Handle, "\nS.ThreadIds = [");
	for(uint32_t i = 0; i < S.nNumLogs; ++i)
	{
		if(S.Pool[i])
		{
			MicroProfileThreadIdType ThreadId = S.Pool[i]->nThreadId;
			if(!ThreadId)
			{
				ThreadId = (MicroProfileThreadIdType)-1;
			}
			MicroProfilePrintf(CB, Handle, "%d,", ThreadId);
		}
		else
		{
			MicroProfilePrintf(CB, Handle, "-1,", i);
		}
	}
	MicroProfilePrintf(CB, Handle, "];\n\n");

Btw, that else block looks wrong, what's that i doing there?

Oh I see the actual problem is that the rest of this stuff is still 0:

S.tt_0_1 = [1,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0];

[ZenoArrows]

Ok, I've figured it out, some platforms are more permissive with the printf format string than others. MicroProfile is using 64-bit integers with %d instead of %lld, which results in undefined behavior causing some implementations to simply write a 0. Also there are other conflicts like using %d instead of %u with unsigned integers.

Oh wait, MicroProfile uses stb_sprintf, I wonder why there's a difference in behavior (the fact the PS Vita is a 32-bit platform is probably related). In any case even stb_sprintf requires using %lld or %I64d for 64-bit integers.

[ZenoArrows]

I actually have another suggestion, according to the commit history the dependency on stb_sprintf was added because of locale issues with the libc sprintf (and changing the locale is not thread-safe). Another option is to switch over to STL IO stream functions which allows a stream to be imbued with a locale. The STL streams are quite a bit slower than sprintf, but it won't have any of the typing issues given everything is simply derived from the variable type. It'll also allow microprofile to return to just being a single header and an implementation file.

[jonasmr]

Hey - I haven't looked at your changes yet, but I cam come with a quick comment: I wont be merging any STL stuff.

I do want your fixes though. I'll get to them asap.

[ZenoArrows]

Understandable on the STL stuff, though in that case I do think it would be nice to have an option that allows the developer to configure microprofile to use the libc sprintf with a big warning on top that you should only enable it if you're certain your program uses the C locale.

[jonasmr]

Hi.

I merged your stuff, thank you!

I'm not sure I understand what the motivation for using the libc sprintf would be? It seems to me using stb would be a better idea because you avoid all the locale stuff?

[ZenoArrows]

It's just to reduce external dependencies in case the locale issues are not a concern. The locale only becomes an issue if the program switches to a locale other than the default C locale.

If you're not interested in offering that as an option then we should replace all instances of %PRIu64 with %lld, since the former is only required for cross-platform compatibility with the libc sprintf.

The main reason I'm asking is that I'd like to create a PR that addresses all of the type correctness issues and makes the format strings consistent.

[jonasmr]

I think the best solution here would be to stick with stb, and not provide the libc option, and going for the %lld everywhere option.

Supporting both would probably end up becoming a mess.

[ZenoArrows]

Works for me, let's clean it up then and use %lld everywhere since we don't need to worry about libc portability.

[jonasmr]

Awesome!