-
Notifications
You must be signed in to change notification settings - Fork 332
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support 1D list as Header init
#1274
Comments
Is there any additional information I can provide here? I'm also happy to do some of the implementation work too |
I was kind of curious how it was more efficient and why people might want to use this. It would also be interesting to know if people today have this kind of data structure and end up having to convert it to suit the (Also, to be clear, is the data model essentially |
This data structure comes from Node actually: https://nodejs.org/api/http.html#http_message_rawheaders It is how headers are currently managed in http. The efficiency I've measured is in overall speed. In my implementation, insert and look up are ever so slightly slower because of the binary search/input algorithm. And by sorting on insert, iteration calls are significantly faster. Additionally, it is theorized that a flat array based structure is more memory efficient as well; though, I have not been able to verify this (if anyone has ideas, please share). |
In that case I think this needs a clearer developer need to move forward. E.g., libraries working around the lack of this in the current constructor or it being a frequently asked question on Stack Overflow or some such. Happy to keep this open so people can find this and chime in though. |
One thing that comes to mind: SSR compat as we add fetch to Node core. We don't want to break existing Node user's code (I.e. deprecating array based raw headers), but would also want to support the fetch api. And unfortunately I don't have direct sources for this, but whenever I teach folks differences between http in the browser and http in Node I have to cover things like this. "Headers are a class apart of the fetch api in your browser, but in node they are just an array of strings" |
While developing undici-fetch and subsequently adding a fetch-based Headers class to Node.js, we discovered that a 1D list of header entries is very efficient.
The implementation developed optimizes across multiple use cases (insert, iteration, and look up) so that any use of the instance is still efficient.
Based on this comment thread in the Node.js PR, I anticipate this kind of change may not land, but I think it would be valuable to discuss anyways.
The proposed change could be as simple as just allowing 1D array as an
init
argument and only adding some extra details to cast it to the existing, 2Dheaders list
backing data structure for the Header api. A more complex (and assumingely unlikely) change would be to update the spec to utilize the 1D based implementation proposed to the Node API.The text was updated successfully, but these errors were encountered: