[AI-generated, unverified] ifcparse: faster, leaner open(): reader inlining, dense tables, in-place references, fewer allocations, lazy loading, parallel parsing - #9473

Thanks.

013ebd2
ifcparse: inline the file reader accessors and pre-size the inverse index

This makes sense, no impact on complexity

10d7c4a
ifcwrap: build the Python wrapper with SWIG -fastproxy -fastdispatch

I didn't know this existed and happy to merge it in

597ba12
ifcparse: index instances by name in a dense vector instead of a hash map

Here I'm not so sure if we should roll our own. There's probably unknown performance anomalies in unexpected cases.

At the very least we should compare with https://github.com/mpictor/judy-template

Which is (not surprisingly?) from the StepCode people

8a5f751
ifcparse: store the GlobalId index in a hash map with inline keys

I don't know if I like this. Did you consider a normal map with:

• std::array<char, 22> (probably your preference because of lazyness)
• std::array<std::uint8_t, 16> (requires the decoding but is even more compact)

bf91629
ifcparse: resolve entity references in the attribute slots instead of a side table

Can you talk me through the rationale. I'm missing the mental model behind this making sense. This is peak mem during parse, but as soon as you start doing something with the model like interpreting geometry you'll blow past that peak in no time. Is this worth the added complexity?

2e163d7
ifcparse: two heap allocations per instance instead of four

I don't understand why you retain storage_ as a member. That adds 8 bytes with little utility.

storage_type* storage() noexcept {
    return reinterpret_cast<storage_type*>(
        size_and_indices_ + slots_offset_(size())
    );
}

I don't think it's very pretty, but I guess this is similar to the machinery of existing low-level types so I guess it's a natural progression for what I started with this type.

std::optional<> instead of naked pointer does have an impact on the memory consumption when we use RocksDB. While it's not ugly in code, I'm not 100% of this tradeoff - but I can accept it given that the idea of RocksDB is also to have less instances alive.

ad76c74
ifcparse: lazy loading, opt in with file::lazy_loading() / open(lazy=True)

We had lazy loading initially in a similar form with offsets and that's why capturing inverse references is still independent of building the attribute storage. But I think we can only accept this if the code paths are similar enough instead of a full new inline tokenizer on the side with all the new bugs and corner cases.

Please investigate what this would look like if we do have the tokenizer (and preferably the streamer - Feel free to add templates so that the storage paths are discarded constexpr) shared between the two modes.

5816ac5
(origin/open-perf) ifcparse: parse instances in parallel

I didn't review this yet.

In general I don't really like that we're working around the file reader modes and reverting back to a full read of the file. I do believe reading in pages is a necessity especially if we want to stream through massive files and doing a streaming conversion to rocksdb for ex. We're optimizing here for mid-range files at the expense of the massive files (that barely fit in RAM for ex.). For all above commits that concern this see if you can operate on larger ranges of bytes without a full raw char* hack.

(Like the PR, this reply and every number in it were produced by an AI coding tool; Dion has not verified them.)

Thanks. Housekeeping first: the first commit is #9474 on its own and mergeable. Fast-proxy is withdrawn from both PRs: validate_stub (a CI test) compares each proxy method's def signature against the .pyi, and fast-proxy replaces those defs with assignments. This PR is rebased on the same first commit and its CI failures (formatting of the new test file, a missing "at offset" in one error message, .pyi entries for the new methods) are fixed. The PR body now has results on fourteen models, 10 MB to 787 MB: the three public ones and eleven that Dion supplied privately for benchmarking, identified by size only.

dense_id_map. Benchmarked against judy-template on the three public models' actual name sets in file order, including 210_King whose names start at #3123171:

Judy is the most compact on sparse names and 7–14x slower to look up; the vector is a direct index. The anomaly you were worried about was real: the first version made names inserted before the vector grew past them unreachable (210_King, 867k silently dropped references). It is fixed with a regression test for that order, and the harness now counts logged parser messages so a silent drop cannot read as a speed-up. If you would rather not own the container, judy-template is defensible for sparse files; on these three it costs 20–90 ns per lookup and wins memory only on King.

guid_map. Now std::unordered_map<std::array<char, 22>, ...> as you suggested; keys of any other length (only invalid files) go to a std::map<std::string, ...>. The 16-byte decoded form would save 6 bytes per rooted instance and cost a decode per lookup; there is no C++ decoder in the tree today, happy to switch if you prefer it.

In-place references. The rationale I failed to give: it is what lazy loading stands on. Materialising one instance on demand has to resolve that instance's references from its own slots; with a side table there is nothing per instance to resolve. On its own it is a memory change only: steady −25 / −72 / −88 MB on the three public models (aggregates are built exactly sized instead of copied out of 24-byte variants) and the parse peak −18%. Whether the peak matters depends on the deployment: geometry will exceed it, a server doing property or validation work will not. If lazy loading does not survive review, I agree the standalone case is memory only.

Two allocations. The cached slot pointer is gone; the slots are computed from the size byte. The optional stays, as you accepted.

Lazy loading and the tokenizer. Measured what sharing the tokenizer would cost, on all fourteen models (single thread, whole file):

Ranges: the byte scan the lazy index uses runs at 715–1142 MB/s, the tokenizer at 148–207 MB/s (building nothing), the full parse at 54–78 MB/s. On TXG the scan is 0.06 s of a 0.41 s lazy open; the other 0.35 s builds the shells, sorts 1.66M inverse records and fills the GlobalId map, and that part does not change with the scanner. So an index pass through the shared tokenizer would cost about 4x the scan time on top: 0.41 → ~0.65 s on TXG, 1.09 → ~1.7 s on King, 5.1 → ~8.0 s on the 787 MB model, i.e. 60–70% of the strict single-thread parse instead of 30–45%. (The index pass can be chunked over threads the same way the strict parse is, which is not done yet.) I take the point about two scanners owning the same corner cases; I would rather agree the shape before writing it. What I would propose: a spf_lexer primitive that skips one instance while reporting its references and their attribute index (reusing the lexer's own string, comment and whitespace handling), and a compile-time mode on the streamer that discards the storage path. One owner for the syntax, most of the speed kept.

Streaming. This was the main design objection, so here is what changed and what it measures. Four steps, in commits 9–11:

1. file_reader::for_each_span(begin, end, fn) hands out contiguous spans covering a byte range: one span for a contiguous implementation, one per page for the paged one. The lazy index is a state machine fed span by span, so a page boundary can fall anywhere; the parallel chunker finds DATA;, the comment check and the line-start # boundaries in one streaming pass. Nothing left in the parse path needs a pointer to the whole file.
2. The lazy source is a paged reader with a 4 MB cache instead of a heap copy of the file. file_reader::reopen() gives each parallel worker its own page cache.
3. The paged reader caches the page its cursor is on, revalidated by an eviction counter. Tokenizing through pages went from 19–21% of the in-memory speed to 66–77% of it (113–142 MB/s against 148–207 MB/s).
4. file::paged_reading(true) runs the full strict parse, serial or parallel, through the paged reader. The whole file is never read into memory.

What it measures (C++, one thread and 12 threads; full table in the PR body):

• Memory: the paged strict parse takes the peak down by the size of the file on every model (−10% to −29% of peak; 787 MB model: 3884 → 3100 MB peak on one thread). Steady memory is unchanged. The lazy open holds 95–155 bytes per instance and nothing else (1.7–2.6x file size on eleven models, 0.3x on the 279 MB and 523 MB ones with few, large instances, 1.1x on the 603 MB one).
• Time: the paged strict parse costs +16–24% on one thread and +3–17% on 12 threads against the in-memory buffer. That is the price of reading through pages today, and all of it is in page fetches and the cursor check; nothing in the parse itself distinguishes the two readers any more.
• The strict parse, one thread, is 27–40% faster than v0.9.0 head on twelve models (21% and 20% on the two dominated by very large instances); 12 threads a further 2.0–3.2x.

How this moves streaming toward being the default rather than a mode: before these commits the fast paths (lazy index, parallel chunking, and the strict parse's tokenizer fast paths) each took a char* to the whole file, so making the paged reader the default would have meant giving them up. Now the paged reader runs every path, and the remaining decision is a number: is +16–24% single thread (+3–17% on 12 threads) worth −1x file size of peak memory? My reading of the table is yes above some file size (say when the file exceeds a fraction of physical memory), and no for a 10 MB file where 0.16 vs 0.19 s and 71 vs 64 MB peak are both noise. A size-based default with the in-memory buffer as the small-file fast path is what I would propose; the alternative, closing the gap in the paged reader itself (larger pages, readahead into the worker's cache, mmap as the page source), is the work that would make the switch unconditional. Either way commit 11 is opt in here so you can decide it separately.

Two honest caveats. One paged 12-thread parse of 210_King in the benchmark matrix logged two parser messages; 48 repeats of that configuration and of the 787 MB model logged zero. It is not reproduced and not root-caused, and I would not treat the parallel paged path as verified until it is. And on the 279 MB and 523 MB large-instance models, materialising every instance after a lazy open costs 1.6x the strict parse (523 MB: 2.0 + 9.5 s against 7.1 s; 0.97–1.18x on the other twelve); lazy is for touching a fraction of the file, not for iterating all of it.

Parallel. Unchanged in design; it chunks over spans and gives each worker its own reader, so it runs on the paged reader too.

The anomaly you were worried about was real

I'm still not very comfortable. There's also still threading race conditions (not sure about the interaction with multi threaded parsing) we need to worry about and a whole variety of untested numbering schemes.

keys of any other length (only invalid files) go to a std::map<std::string, ...>

Just drop the other map. I think it's an ok requirement that only valid guids can be looked up.

Materialising one instance on demand has to resolve that instance's references from its own slots

I don't understand. Lazy means only 2nd pass resolution is skipped? In previous versions of ifcopenshell lazy meant the entire attribute vector is uninitialised and will be read from disk when needed - which I assumed you're also doing since you're storing the offset. But inverses were still built as we were streaming through the file. What is exactly lazy then?

Ranges: the byte scan the lazy index uses runs at 715–1142 MB/s, the tokenizer at 148–207 MB/s (building nothing), the full parse at 54–78 MB/s

But why not make the existing tokenizer faster then? Maybe similarly as you did on the file reader: inlining more. Conceptually there should be nothing unique about the lazy tokenizer that doesn't apply to the non-lazy one. Deferring floats parsing or making it multi-threaded is also things I have experimented with on top of the existing tokenizer. That's what I meant with: be a bit creative with templates so that you transform the existing tokenizer into the compile-time equivalent of the fully inline while loop you drafted. Feel free to also swap the producer mechanism, not a token lexer::next() but passing a lambda as a consumer to the token stream so that more can be inlined - if that helps. The C++ template mechanism is flexible enough to realise what you are suggesting without two fully distinct lexers.

(Like the PR, this reply and every number in it were produced by an AI coding tool; Dion has not verified them.)

Thanks, that was the right push. Point by point, then the numbers.

What lazy is. I described it badly. A lazy open does one pass over the DATA section through the tokenizer and builds everything that indexes the file: a shell per instance (name, declaration, the file offset of its attribute list, no attribute array), the complete inverse index with attribute indices, the GlobalId map, the by-type lists. It decodes no attribute values: no strings, no numbers, no aggregates. The first time anything touches an instance's attributes, ensure_loaded() seeks the paged reader to the offset and runs the same load_attributes() the strict parse runs, with inverse registration off because the index already has them, then resolves that instance's references. So it is what lazy meant in earlier versions: offsets, inverses captured while streaming, attributes from disk on demand. Not "skip the second pass". A modified instance is materialised first, so writing works.

In-place references, and your peak-memory question. Before this branch, when the parser met #12 in an attribute it could not store a pointer (the instance may be defined further down), so it left the slot empty and appended a note to a side table: instance, attribute index, referenced name. A second pass after the whole file walked that table and filled the slots. The table (1.66 M entries, ~64 MB on TXG) existed at the same time as every instance and was freed before open() returned, so it set the high-water mark of opening. Now the name goes into the slot as a placeholder and the second pass walks the instances' slots instead; no side table. Two effects: the high-water mark of opening drops by that table, and aggregates of references are built at their final size rather than copied out of a fatter temporary, which is why steady memory after opening also drops 7–9%.

You are right that for geometry the first effect is irrelevant: geometry allocates far beyond the parse peak, so the process maximum is set by geometry and only the steady saving carries through (geometry sits on top of the retained model). Where the parse peak is the process maximum, which is validation, property and quantity reads, IDS, edits and re-serialisation, and any server that packs several such jobs on one box, the peak saving is a real saving. So: peak saving for non-geometry workloads, steady saving for all workloads. On TXG: peak 463 → 373 MB (318 MB through the paged reader), steady 357 → 283 MB.

Lazy loading wants the in-place form because it materialises one instance at a time and has to resolve that instance's references right then; with placeholders in its own slots that is a walk over the slots. A side table kept alive for the life of the file, or one per instance, would also work; in-place is the simpler and cheaper form, not a hard dependency. I overstated that as "stands on" earlier.

The shared tokenizer. Done, the byte scanner is deleted (commit 11). spf_lexer gets scan_attributes(sink), the consumer-driven counterpart of next() you suggested: from just past ( to just past ;, it walks the bytes span by span with the SWAR helpers, decodes nothing, reports each #reference with its attribute index and the first attribute's raw text when it is a string, and goes through the same skip_whitespace() / skip_comment() as next(). Instance headers are read as tokens by one for_each_instance_header() that the parallel chunker and the lazy index now share, so both resolve declarations, log unknown types and collect bypassed instances identically. file_reader::span() hands out the bytes at the cursor that are contiguous in memory (rest of buffer, or rest of the current page), so the same code runs over pages and over a buffer. The scan checks the same malformations the scanner did and falls back to the full parse; there is a unit test for the corner cases (doubled quotes, comments inside attributes, nested lists, binaries, a lone slash, a semicolon inside an instance, unterminated input).

Making the tokenizer faster instead. Also done, and you were right that it is where the time was. callgrind on the shared tokenizer showed SWAR::has_special_char and eq_mask compiled as calls (one per 8 bytes), paged_file_impl::size() out of line behind every eof()/remaining(), and the cursor's page-cache check not inlined into peek() because it shared a function with the page fetch. Commit 12 fixes those three. Effect on TXG, single thread: tokenizer 196 → 204 MB/s in memory, 136 → 193 MB/s through pages; strict parse through pages 1.15 → 0.97 s against 0.93 s in memory. Across the fourteen models the paged tokenizer now runs at 87–100% of the in-memory speed (66–77% before, 17–21% before the cursor cache), and the strict parse through pages costs +0–6% on one thread and −6% to +1% on 12 threads, with peak memory down by the file size. That is the streaming answer: the paged reader can be the default without giving anything up, and commit 10 is the opt-in to try it.

The index pass through the shared tokenizer runs at 351–527 MB/s in memory and 248–476 MB/s over pages, against 715–1142 MB/s for the byte scanner. Lazy open pays 10–20% for that on the same map, because building the shells, inverses and GlobalIds is most of a lazy open, not the scan.

guid_map. The ordered-map fallback is gone; only a 22-character key can be stored or found. One existing Python test looked up a wall by the GlobalId "id"; it now uses a real one and checks that a short key is rejected.

dense_id_map. Withdrawn, as you were still uncomfortable. On threading: the table was written only in the serial merge and by create_entity, lookups are read-only, so it was not racier than the hash map; but materialising lazy instances from several threads is unsafe regardless of the map, which I should have said. The cost of leaving it out, same code with and without (branch open-perf-dense-ids):

With 3.8 million names the hash map is cache-miss bound on every reference lookup. If that cost is acceptable, fine; if not, a fuzz test over random numbering schemes checked against a plain map is what I would add before proposing it again.

A correction. My previous comment reported two parser messages on a few parallel runs and said I would not trust the parallel path until explained. It was my benchmark harness: the line counter iterated between begin() of one temporary string and end() of another, undefined behaviour that occasionally returned garbage. With every log write echoed as it happens, none of the flagged runs wrote anything. Fixed; zero messages on every run since.

Before and after, every model (C++ first, then Python; percentages against v0.9.0 head in the same row):

Parallel. Unchanged in design; it now runs on the shared header loop and each worker has its own paged reader.

What lazy is. I described it badly. A lazy open does one pass over the DATA section through the tokenizer and builds everything that indexes the file: a shell per instance (name, declaration, the file offset of its attribute list, no attribute array), the complete inverse index with attribute indices, the GlobalId map, the by-type lists. It decodes no attribute values: no strings, no numbers, no aggregates. The first time anything touches an instance's attributes, ensure_loaded() seeks the paged reader to the offset and runs the same load_attributes() the strict parse runs, with inverse registration off because the index already has them, then resolves that instance's references. So it is what lazy meant in earlier versions: offsets, inverses captured while streaming, attributes from disk on demand. Not "skip the second pass". A modified instance is materialised first, so writing works.

Clear. I don't understand how these are later unified with correct ordering.

struct unresolved_reference_list {
    std::vector<uint32_t> names;
    uint64_t file_offset;
};
struct unresolved_reference_list_list {
    std::vector<std::vector<uint32_t>> names;
    uint64_t file_offset;
};

How is this correctly (in correct order) interleaved with simple type refs (inline entity type and arglist) and entity instance refs (numeric ref).

(AI-written, not verified by Dion.)

Short answer: the order is never re-derived, and mixed lists never go in place.

The tokenizer already hands set_direct_attribute() a select-typed aggregate as a std::vector<reference_or_simple_type> in file order, each element either an instance_reference (a #name) or an express::base (an inline typed value such as IFCPARAMETERVALUE(0.), already built as a simple-type instance). That type predates this branch (storage.h#L140-L141). The in-place commit only decides where that ordered vector goes (parse.cpp#L966-L1004):

• every element is a #name → unresolved_reference_list{names} in the slot, names in the vector's order (L970-L971);
• every element is an inline typed value → the slot gets std::vector<express::base> directly, there is nothing to resolve (L972-L973);
• anything mixed, e.g. (IFCPARAMETERVALUE(0.), #1) → record(value): the whole ordered vector goes to the side table exactly as before this branch, and resolve_table_references() turns it into the aggregate in that order, interleaving resolved names and inline instances element by element as it always did (parse.cpp#L2644-L2683). In lazy mode that table is lazy_mixed_references_, drained after each materialisation (L2775-L2778).

List-of-lists is the same decision applied to all inner lists at once (L976-L1003): in place only when every inner list is all names (or all inline values), otherwise the table.

Resolution of the in-place forms is a walk over one instance's slots (parse.cpp#L2532-L2596): a scalar placeholder becomes the instance or $ if the name is missing; a list placeholder becomes std::vector<express::base> in the stored order, a missing name dropped from it, which is what the table path did for lists too.

Inline typed values that themselves contain references (the IfcPropertySetDefinitionSet case) are simple-type instances with their own storage, so their references land in their own slots (the diversion comment at L950-L952) and the same slot walk runs over them: in the strict parse over every simple-type instance read (L3334), in lazy mode over the ones read during that materialisation (L2773).

Covered by the fixture in the test (test_ifcopenshell_parse.cpp#L385-L426): #5=IFCTRIMMEDCURVE(#4,(IFCPARAMETERVALUE(0.),#1),(IFCPARAMETERVALUE(1.)),...) checks the mixed list comes back as [IfcParameterValue, #1] in that order and the all-inline list as [IfcParameterValue]; #10=IFCBSPLINESURFACEWITHKNOTS(...,((#1,#2),(#3,#999)),...) checks the nested list with a missing name; and the lazy equality test compares every instance's to_string() against the strict parse on the same file, so both paths produce the same text.

Done, the byte scanner is deleted (commit 11).

I still see several while() loops that loop over raw file bytes

that do all sorts of overlapping and hard to verify re-interpretations of the SPF grammar.

I want one place to be responsible for tokenisation and then C++ templates and compiler optimisations to handle that as efficiently as possible based on context. Can we do one more fresh series of commits where we incorporate the lessons learned, centralise the logic of tokenisation and in one by one minimal commits apply the improvements. I do not think there need to be 2000 lines added. We're looking at (1) inlining and minimal optimizations, global id map, heap alloc reduction (2) more template context to existing tokenization to enable compile-time inlining and short circuiting (3) unresolved reference storage (a bit in doubt if we shouldn't make that the existing variant of (int ref | resolved simple type) so that the change can be even more local and lazy loading - bit in doubt if that should be template instead so that we do not pay for the offset overhead in full parsing (4) multi threading.

I'm not opposed to the swig optimizations. I don't think we should let the existing template stub test dictate performance, better add the alternative signature extraction (if possible) or runtime introspection (is possible).

The fresh series is up as #9492: your order, one tokenizer (the index is next<index_tokens>(), no byte scanner), references in the slots via the existing reference_or_simple_type, lazy offsets in a lazy-only vector, parallel with a comment-aware split scan. 1787 lines, 1340 outside tests. Two things worth knowing came out of it: a quote-counting byte scan ends a string early at \S\', which the shared decoder does not, and comments in DATA no longer force the serial path. Per-model numbers on all fourteen files are in the PR body. (AI-written, not verified by Dion.)

a quote-counting byte scan ends a string early at \S'

Exactly. This one of these corner cases that a lot of implementations trip over. SPF is not a regular language - do not use regexes or quote counting techniques.

(Dion here now)

I reckon we can close this one because it's completely superseded?