Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MAYA-104552 improve diagnostics #2799

Merged
merged 3 commits into from
Jan 13, 2023
Merged

Conversation

pierrebai-adsk
Copy link
Collaborator

Improve the message batching to be usable for errors too. Also, remove
hard-coded crash when we can. We should never explicitly choose crash Maya
under the user's feet just because of some-sub-system is unhappy. Errors are
fine for those cases.

Unfortunately, there are some messages that comes from OGS and which require a
Maya fix to avoid flooding the script editor, those cannot be fixed in the
plugin. (Well, it would need investigation why the VP2 delegate does what it
does and causes those OGS errors.)

  • Always accumulate and coalesce messages.
  • Use a thread to periodically write messages.
  • Only write messages if there are some pending and at least a second has passed.
  • For low-volume messages, this will print all messages.
  • For high-volumes messages, this avoids a flood of messages.

The design goes like this:

  • All messages are accumulated by the diagnostic delegates.
  • Another delegate tells the diagnostic message flusher when
    any message arrives.
  • The diagnostic message flusher has two purposes:
    • the first purpose is to detect bursts of messages and to withhold
      further messages from being written out when a burst is detected,
    • the second purpose is to write out (flush) the messages periodically.
  • The condition for flushing are either:
    • that a forced flush is requested,
    • or that fewer than a maximum consecutive messages have been received,
    • or that one second has elapsed since the last time messages were flushed.
  • Flushing can either be immediate or delayed.
    • Immediate flushing is done when a forced flush is requested or when
      outside of burst of messages.
    • Delayed flushing is done when a burst of messages is detected,
      to avoid writing too many messages in the log.
  • Requesting a flushing of accumulated messages is done either directly
    or indirectly.
    • Direct flushing is done when the flushing request is triggered in the
      main thread.
    • Indirect flushing is done by queuing a task to be run on-idle in the
      main thread. If a task is already queued, nothing is done, to avoid
      queuing multiple redundant tasks to do the same thing.
  • The actual flushing takes (extract and removes) all accumulated messages
    and prints them in the script console via MGlobal.
  • This can only be done in the main thread due to the limitations of MGlobal.
  • Printing of messages is done either fully or coalesced.
    • All messages are printed fully when not in a burst.
    • All messages are printed coalesced when in a burst. Coalesced messages
      only print a sample of the message followed by "and X similar".

Improve the message batching to be usable for errors too. Also, remove hard-coded crash when we can. We should never explicitly choose crash Maya under the user's feet just because of some-sub-system is unhappy. Errors are fine for those cases.

Use the diagnostic batch context in places that caused problems due to the flood of error messages when loading large scene with missing textures or layers.

Unfortunately, there are some messages that comes from OGS and which require a Maya fix to avoid flooding the script editor, those cannot be fixed in the plugin. (Well, it would need investigation why the VP2 delegate does what it does and causs those OGS errors.)
- Always accumulate and coalesce messages.
- Use a thread to peridodically write messages.
- Only write messages if there are some pending and at least a second has passed.
- For low-volume messages, this will print all messages.
- For high-violumes messages, this avoids a flood of messages.

The design goes like this:
- All messages are accumulated by the above delegates.
- A thread, periodicFlusher, wakes up every second to conditionally flush pending messages.
- The condition for flushing are that a forced flush is requested or some messages have been received and one second has elapsed.
- Requesting a flushing of accumulated messages is done by queuing a task to be run on idle in the main thread. If a task is already queued, nothing is done.
- The main-thread task takes (extract and removes) all accumulted messages and prints them in the script console via MGlobal.
- This can only be done in the main thread because that is what MGlobal supports.
- Only start to batch message if more than a threshold are received within one second.
- Flush directly if flushing is triggered in the main thread.

Use the batching context to force a flush of diagnostic messages in unit tests that assert about messages.
@pierrebai-adsk pierrebai-adsk added enhancement New feature or request adsk Related to Autodesk plugin labels Dec 28, 2022
@pierrebai-adsk pierrebai-adsk changed the title Bailp/maya 104552/improve diagnostics MAYA-104552 improve diagnostics Jan 6, 2023
Copy link
Collaborator

@vlasovi vlasovi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

@pierrebai-adsk pierrebai-adsk added the ready-for-merge Development process is finished, PR is ready for merge label Jan 11, 2023
@seando-adsk seando-adsk added core Related to core library and removed adsk Related to Autodesk plugin labels Jan 13, 2023
@seando-adsk seando-adsk merged commit af81772 into dev Jan 13, 2023
@seando-adsk seando-adsk deleted the bailp/MAYA-104552/improve-diagnostics branch January 13, 2023 16:19
@dj-mcg
Copy link
Collaborator

dj-mcg commented Jan 20, 2023

Hey folks, we've stopped receiving TF warnings and errors in our unit test log files if we don't perform a diagnostic flush on teardown. I tried setting the PIXMAYA_DIAGNOSTICS_BATCH env var to false but that didn't do the trick. I'm wondering what the method is to completely disable batching of diagnostics because in these cases it seems like we want them as they are issued - thanks!

@pierrebai-adsk
Copy link
Collaborator Author

pierrebai-adsk commented Jan 20, 2023 via email

@pierrebai-adsk
Copy link
Collaborator Author

Oh, wait, you are right, due to the way the message are buffered now, you might indeed need a flush. We probably should register an exit callback in the diagnostic delegate do it automatically or support not batching any messages, you are right.

@dj-mcg
Copy link
Collaborator

dj-mcg commented Jan 20, 2023

Ah ok - it would be great to ensure messages are flushed on exit so we wouldn't have to introduce any new env vars or function calls to our unit tests, but being able to explicitly disable batching also seems like a good idea... Thanks!

@pierrebai-adsk
Copy link
Collaborator Author

pierrebai-adsk commented Jan 20, 2023

I've looked at the code. I could try to fix it, but given that I don't have repro step to make sure I've fixed the issue in your particular case, I'll describe what could be modified. You could tried it on your side and submit a PR.

  1. In the DiagnosticFlusher constructor, check PIXMAYA_DIAGNOSTICS_BATCH and make the _maximumUnbatchedDiagnostics some huge number (i.e. INT_MAX).
  2. Add a Maya-Exit callback to call UsdMayaDiagnosticDelegate::Flush(). Normally it gets called when the plugin is unloaded when Maya exits, but in release builds of Maya, Maya uses what it calls fast-exit, which skips a lot of cleanups and probably explains why the flush is not automatically done. Even in fast-exit, the exit callbacks do get called, so that would ensure the messages are flushed.

I think #2 would be enough to ensure flushing in your case, but #1 could be a nice extra.

@pierrebai-adsk
Copy link
Collaborator Author

I went ahead I wrote what I described in the PR: #2830

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Related to core library enhancement New feature or request ready-for-merge Development process is finished, PR is ready for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants