Skip to content

Conversation

@eivindjahren
Copy link
Collaborator

This is by using slots for arrays. This seems to give a very minor improvement to performance.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR aims to improve memory usage of array entries by adding __slots__ to the ResArray class. The __slots__ mechanism prevents Python from creating a __dict__ for each instance, which can significantly reduce memory overhead when many instances are created.

Key Changes:

  • Added __slots__ definition to ResArray class containing all instance attributes: start, stream, _keyword, _length, _type, _data_start, and _is_eof

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@@ -25,10 +25,6 @@ class FormattedArray(ResArray):
An array entry in an formatted res file.
"""
Copy link

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

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

The FormattedArray subclass should define __slots__ = () to prevent the creation of __dict__ in instances. When a base class uses __slots__, subclasses that don't explicitly define their own __slots__ will still have a __dict__ attribute, which defeats the memory optimization purpose of using __slots__ in the base class. Add an empty __slots__ = () declaration to this class.

Suggested change
"""
"""
__slots__ = ()

Copilot uses AI. Check for mistakes.
Copy link

@andreas-el andreas-el left a comment

Choose a reason for hiding this comment

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

💯

@andreas-el andreas-el merged commit 5c68bd5 into main Dec 12, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants