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

Disable spilling in LastWideCombiner if input or state is not serializable (currently has Resource type). #4590

Merged
merged 2 commits into from
May 16, 2024

Conversation

Darych
Copy link
Collaborator

@Darych Darych commented May 16, 2024

Changelog entry

Disable spilling in LastWideCombiner if input or state is not serializable (currently has Resource type).

Changelog category

  • Bugfix

Additional information

...

@Darych Darych requested a review from lll-phill-lll May 16, 2024 09:16
@Darych Darych requested a review from a team as a code owner May 16, 2024 09:16
@@ -1523,6 +1530,10 @@ using TBaseComputation = TStatefulWideFlowCodegeneratorNode<TWideLastCombinerWra
#endif
};

bool IsTypeSerializable(const TType* type) {
return !type->IsResource();
Copy link
Member

Choose a reason for hiding this comment

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

As far as I understand there are more non-serializable types.
Here are all possible types:

#define MKQL_TYPE_KINDS(XX) \

And all the types that are not listed here are non-serializable:
bool HasOptionalFields(const TType* type) {

Also, I think we can use ctx.Mutables.SerializableValues to get indexes of serializable values

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thank you and I agree with you. Will add.

Copy link

github-actions bot commented May 16, 2024

2024-05-16 09:51:05 UTC Pre-commit check for 9d7273c has started.
2024-05-16 09:51:06 UTC Build linux-x86_64-release-asan is running...
🟢 2024-05-16 09:53:43 UTC Build successful.
2024-05-16 09:55:22 UTC Tests are running...
🔴 2024-05-16 11:41:48 UTC Some tests failed, follow the links below.

Test history

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
11703 11616 0 29 50 8

Copy link

github-actions bot commented May 16, 2024

2024-05-16 10:46:00 UTC Pre-commit check for 9d7273c has started.
2024-05-16 10:46:01 UTC Build linux-x86_64-release-clang14 is running...
🟢 2024-05-16 10:48:58 UTC Build successful.

Copy link

github-actions bot commented May 16, 2024

2024-05-16 10:46:05 UTC Pre-commit check for 9d7273c has started.
2024-05-16 10:46:06 UTC Build linux-x86_64-relwithdebinfo is running...
🟢 2024-05-16 10:48:57 UTC Build successful.
2024-05-16 10:50:32 UTC Tests are running...
🔴 2024-05-16 12:37:38 UTC Some tests failed, follow the links below.

Test history

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
70528 57853 0 1 12657 17

Fix overriding allowSpilling flag.
Copy link

github-actions bot commented May 16, 2024

2024-05-16 13:37:06 UTC Pre-commit check for a084d8e has started.
2024-05-16 13:37:08 UTC Build linux-x86_64-release-asan is running...
🟢 2024-05-16 13:39:44 UTC Build successful.
2024-05-16 13:41:28 UTC Tests are running...
🔴 2024-05-16 15:26:27 UTC Some tests failed, follow the links below.

Test history

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
11705 11581 0 38 77 9

Copy link

github-actions bot commented May 16, 2024

2024-05-16 13:53:30 UTC Pre-commit check for a084d8e has started.
2024-05-16 13:53:31 UTC Build linux-x86_64-release-clang14 is running...
🟢 2024-05-16 13:56:32 UTC Build successful.

Copy link

github-actions bot commented May 16, 2024

2024-05-16 13:56:15 UTC Pre-commit check for a084d8e has started.
2024-05-16 13:56:16 UTC Build linux-x86_64-relwithdebinfo is running...
🟢 2024-05-16 13:59:22 UTC Build successful.
2024-05-16 14:00:58 UTC Tests are running...
🔴 2024-05-16 15:54:19 UTC Some tests failed, follow the links below.

Test history

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
70534 57847 0 13 12664 10

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants