Skip to content

Header read-only - #21

Open
Ghesselink wants to merge 1 commit into
IfcOpenShell:masterfrom
Ghesselink:only-header-partial-read
Open

Header read-only#21
Ghesselink wants to merge 1 commit into
IfcOpenShell:masterfrom
Ghesselink:only-header-partial-read

Conversation

@Ghesselink

Copy link
Copy Markdown
Contributor

only_header limits what is returned, not what is read: currentlyparse() reads the whole file and the comment-stripping regex makes a second full copy, before the header (a few hundred bytes, always at the top per ISO 10303-21) is sliced out.

This PR stops reading after the ENDSEC; that closes the HEADER section (64 KiB chunks).

Some tests: peak RSS of an only_header=True parse:

File size Before After
12 KB 22.6 MB 22.6 MB
70 MB 157.9 MB 22.6 MB
130 MB 275.2 MB 22.6 MB

Also slightly faster on large files, but it was never a time cuonsumer to begin with (0.37 s → 0.19 s at 130 MB).

Testing: test_parser.py tested with VS testfiles from the header validation, outcome is the same. g

@aothms aothms left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't spot anything wrong with it, but I also find it a bit of a weird compromise.

I would say:

  • Either, build our own streaming lexer and terminate as soon as we have a complete header. I think this is overkill
  • Just take the first 1 megabyte (or 10 megabyte if you are crazily careful) unconditionally and operate on that.

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