Investigate if we can add 8 bits to the cell struct without affecting performance #593

Open
opened 4 months ago by dnkl · 2 comments
dnkl commented 4 months ago
Owner

The cell struct currently consists of two fields: attrs and wc, where wc is the character it contains, and attrs its attributes (i.e. bold, italic, underline, colors etc).

Most of the attributes are related to ANSI attributes, and are first set in the vt struct's attrs member, and then copied to the cell when a character is printed.

That is, all cell attributes are overwritten when a character is printed.

This becomes a problem for the few attributes that are not related to the ANSI state: currently url and selected.

The selected bits informs the renderer that the cell is selected (by the user) and should be highlighted as such. Thus, if the user has selected a region of text, and cells within that region is updated, those cells "lose" their selected state. We are currently working around this by walking all selected cells and updating their selected bits just before rendering.

Things would become much simpler if we could separate the ANSI state from cell persistent state. For performance reasons, we cannot just split the attributes struct, as that would require more instructions when copying the attributes when printing a character.

But perhaps we can add an uint8_t to the cell struct? This new bitfield would contain state that needs to be persistent across cell writes. It would of course increase foot's memory usage, with ~8% (13 vs. 12 bytes). It may also affect our cache usage in a negative way.

The `cell` struct currently consists of two fields: `attrs` and `wc`, where `wc` is the character it contains, and `attrs` its attributes (i.e. **bold**, _italic_, underline, colors etc). Most of the attributes are related to ANSI attributes, and are first set in the `vt` struct's `attrs` member, and then **copied** to the cell when a character is printed. That is, **all** cell attributes are **overwritten** when a character is printed. This becomes a problem for the few attributes that are **not** related to the ANSI state: currently `url` and `selected`. The `selected` bits informs the renderer that the cell is selected (by the user) and should be highlighted as such. Thus, if the user has selected a region of text, and cells within that region is updated, those cells "lose" their selected state. We are currently working around this by walking all selected cells and updating their `selected` bits just before rendering. Things would become much simpler if we could separate the ANSI state from cell persistent state. For performance reasons, we cannot just split the attributes struct, as that would require **more** instructions when copying the attributes when printing a character. But perhaps we can add an `uint8_t` to the cell struct? This new bitfield would contain state that needs to be persistent across cell writes. It would of course increase foot's memory usage, with ~8% (13 vs. 12 bytes). It may also affect our cache usage in a negative way.
dnkl added the
performance
label 4 months ago
Collaborator

But perhaps we can add an uint8_t to the cell struct?
...
It would of course increase foot's memory usage, with ~8% (13 vs. 12 bytes)

I think adding a uint8_t member would increase sizeof(struct cell) from 12 to 16 bytes. _Alignof(struct cell) is 4, so the compiler would have to add 3 bytes of padding.

> But perhaps we can add an `uint8_t` to the cell struct? > ... > It would of course increase foot's memory usage, with ~8% (13 vs. 12 bytes) I think adding a `uint8_t` member would increase `sizeof(struct cell)` from 12 to 16 bytes. `_Alignof(struct cell)` is 4, so the compiler would have to add 3 bytes of padding.
Poster
Owner

Ah, right, of course. We could always pack it.

But, and I didn't mention this in the description above, a 13-byte struct means we'll be doing unaligned memory accesses. Not sure how much impact that will have. Slower, for sure, but by how much? I guess that's part of the investigation :)

We get by pretty ok with the current state bits. But #592 is hard to do without persistent cell state.

Ah, right, of course. We could always pack it. But, and I didn't mention this in the description above, a 13-byte struct means we'll be doing unaligned memory accesses. Not sure how much impact that will have. Slower, for sure, but by how much? I guess that's part of the investigation :) We get by pretty ok with the current state bits. But https://codeberg.org/dnkl/foot/pulls/592 is hard to do without persistent cell state.
Sign in to join this conversation.
No Milestone
No Assignees
2 Participants
Notifications
Due Date

No due date set.

Dependencies

This issue currently doesn't have any dependencies.

Loading…
There is no content yet.