ml/64 merge develop 92847eb3fd - #1074
Open
matthew-levan wants to merge 90 commits into
Open
Conversation
This reverts commit c79f7b3.
- C2: fold the per-fragment len_w/offset bound into the existing validation in _mesa_req_pact_done - H2: reject non-boq-13 page inits in _mesa_hear_page before computing the leaf count; inject sub-two-fragment messages directly (lev_d < 2) - L6: bound rem_w against the whole field up front in _sift_bits - M1: size the spin buffer without the trailing NUL, mint with u3i_bytes - L5: assert d_reclen fits struct dirent in u3_readdir_r - revert lss.c (H3/M2) and arena.h (L7) - trim security comments; wire bytestream-test and mesa-test into CI
@joemfb I tried replacing u3r_mean calls with explicit core splitting. Some observations: 1. Compiler did eliminate redundant deconstructions; 2. `+ride` call on Hoon source seems ~5% _slower_ with explicit splitting. The way I implemented it was: 1. Define macros that take `u3_noun` or `u3_weak` and return `u3_weak`: ``` #define u3r_head_weak(SOM) ({ \ u3_weak _som = SOM; \ ( u3_none == _som ) ? u3_none \ : (_(u3ud(_som))) ? u3_none \ : ( ((u3a_cell *)u3a_to_ptr(_som))->hed ); \ }) #define u3r_tail_weak(SOM) ({ \ u3_weak _som = SOM; \ ( u3_none == _som ) ? u3_none \ : (_(u3ud(_som))) ? u3_none \ : ( ((u3a_cell *)u3a_to_ptr(_som))->tel ); \ }) #define u3r_head(SOM) ({ \ u3_noun _som = SOM; \ ( _(u3ud(_som)) ) ? u3_none \ : ( ((u3a_cell *)u3a_to_ptr(_som))->hed ); \ }) #define u3r_tail(SOM) ({ \ u3_noun _som = SOM; \ ( _(u3ud(_som)) ) ? u3_none \ : ( ((u3a_cell *)u3a_to_ptr(_som))->tel ); \ }) ``` 2. Apply a [generated python script](https://gist.github.com/dozreg-toplud/f392be7cac99ca8435717b4817fb196a) to all files in `/jets` directory, turning `u3r_mean` into composition of the function-like macros defined above Am I missing some way to make it more efficient?
Happened to use +spin in SKA and decided to check whether it had a jet driver. Apparently it didn't. Didn't measure the difference but the jet gets rid of the +flop call and removes gate calling overhead, so the performance should be much better.
It always felt strange to me that `+silt/+malt` iterate over the list of elements in a loop, assembling an nlr-tree by putting one element at a time. Doing so may cause arbitrary reshuffling of nodes on every put, since there is no guarantee for the order of insertions relative to +gor/+mor orderings. This PR adds a fast path for `+gas:by/+gas:in` if the input tree is empty. The fast path is basically quicksort, where the pivot is chosen by taking a "mor"-most element, and the partions are made with `+gor`. Extra attention was payed to `+gas:by` jet to make sure that for each key only the last key-value pair is taken: order of partitions is preserved, and the last key-value pair is used as a pivot, keys being equal. Tested by setting `ice` of jets to `c3n`, then booting and running the test suite. When comparing performance I saw ~30% improvement for the fast path.
`recover: top: meme` (ie, `u3m_signal(c3__meme)` on the home road) sets the `u3o_check_corrupt` flag, which leads to a full gc in mars after the next global state mutation. This feature, while questionable, does correctly clear up memory leaks (at least in many scenarios) that were caused by abandoning the computation that caused the guard page to be hit. But it doesn't reset the flag, so gc passes are repeated after every subsequent mutation. The PR just unsets the flag after gc. Our overall approach to OOM-on-the-home-road should be reevaluated, as this feature is only active through the `u3m_signal()` codepath -- `u3m_bail()` on the home road is simply a fatal error.
Some refcount bugs caught by a linter (WIP): - `bytestream.c`: we lose retained `octs_list`; - chacha: forgot to increment the refcount of retained `wid`, didn't test `wid` and `dat` for being atoms - hmac: didn't test `out` for being a direct atom even though the code clearly assumes it; missing c3_w -> atom conversions - mat/rub/scow: plain leaks - shax: confusion between retained `c` and owned `d` - slaw: leaks, ~~assumptions that `numname` is a direct atom (so no `u3k(year)`) but no checks;~~ better fix is to just get rid of leaks by not having generic macros - urwasm: leaks in `run_once`, saner control flow which is more readable for a linter and a human; - `u3z_uniq`: leaked `key` - `loss.c`: an atom was used as direct with no check
Consider the following scenario. A client sends a command through the khan socket. Immediately after sending the command the client closes the socket without having time to receive the response. We end up in _conn_moor_bail with a UV_EPIPE and queue another write to the socket that just errored out. Then, in _u3_newt_moat_stop, we call _uv_close and override the bal_f with _conn_moat_free. The event loop turns. The enqueued write gets attempted, immediately hits EPIPE and ends up freeing libuv handle in _conn_moat_free. This is not very good because libuv is not done with the handle yet, you're not supposed to free the handles on error, only on close. So we segfault later inside libuv. The root cause of this issue is that the bal_f is overloaded to mean "failure callback" and "cleanup destructor". It is somewhat unintuitive that calling uv_close can invoke the failure callback again before calling the close callback but such is life in the asynchronous world.
Previously we would be run into use-after-frees in _http_cache_scry_cb and _http_scry_cb.
My previous commit broke the happy path: _http_scry_cb sets the req_u to 0 when it responds and so we will end up dereferencing a null pointer later in _http_req_close. Now we scope the detaching to only occur if _http_req_close happens before _http_scry_cb.
matthew-levan
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.