Use a Set for checking if an element is a void element, which is faster than object access

[NullVoxPopuli]

When it comes to raw speed, for small datasets, Map.get is faster than Object[..] is faster than Set.has

benchmark - Map#get - key exists

benchmark - Map#get - key does not exist

builds on top of: #1467

benchmark - Map#has - key does not exist

benchmark - Map#has - key exists

[fisker]

It's slower on my machine.
You should also test non-exists tags in the benchmark, I think.

[NullVoxPopuli]

good idea.

with non-existent-keys

It looks as if object wins out again.

so I'm going to leave this as an object.

[fisker]

Map#has seems faster then Map#get

[NullVoxPopuli]

that's curious. which makes for an interesting situation with this data structure, because using Map#has, makes the Map basically a Set, but Map performs way better than Set.
I wonder if that means browser vendors just haven't optimized Set as extensively yet?

[chancancode]

Generally in that package we don’t care about performance that much, at least not to that extent, since it typically only runs during the build, and of all the things that is slow in the build, it is not going to come down to these lookups. So just use whatever reads best and makes the most sense.

[chancancode]

Also if this intended to be public then it’s probably better to export a function to check rather than exposing whatever underlying data structure

[NullVoxPopuli]

well, I use @glimmer/syntax at runtime for REPL stuff 😅

[NullVoxPopuli]

Also if this intended to be public then it’s probably better to export a function to check rather than exposing whatever underlying data structure

that's PR #1467

@fisker how does prettier need to interact with this data?

[fisker]

We bundle them to JSON... 😄

[fisker]

And use it in as a Set https://github.com/prettier/prettier/blob/e682577c08e83c0ab272fc73ad1633883ad5d1a5/src/language-handlebars/utils.js#L16

[chancancode]

Yeah then I think we just expose getVoidTags that returns an array and isVoidTag and keep the implementation private

[chancancode]

looks good to me! fwiw, I don't know how "public" anything really is, so at some point, probably when we talk about restructuring the packages to simplify things, we probably want to be more rigorous about delineating the public api surface

[chancancode]

well, I use @glimmer/syntax at runtime for REPL stuff 😅

I still doubt this is what is a hot path that makes it slow, but I can believe it if it shows up on benchmarks; anyway now that the implementation is private you are more free to tweak that if you find that necessary; if it ends up being that important, then I suspect the answer is neither Map nor Set, but perhaps a switch statement in the function where the tags or ordered from the most to least common, and you would probably add fast paths for things like div/p (the most common non-void tags) that returns false early without having to go through the rest.

[chancancode]

But, in general, the performance profiles of these micro things changes over time and we don't have time to keep up with that, so we just end up with a lot of micro optimizations that we don't know if are still necessary or actively causing problems

[fisker]

!! become useless.

[NullVoxPopuli]

thanks! I wasn't even looking at that -- more ancient code

[NullVoxPopuli]

TIL !! in a string in bash is replaced with the previously ran command

[fisker]

Maybe this is a good chance to mention that several void tags actually missing.

https://github.com/prettier/prettier/pull/14110/files#diff-6faed7f5afc0a02a5e775622809f442bc831a1ac00bbb657cd08014962d5aebeR4

Someone may interest to in investigate.

If they are just missing by mistake, maybe we can switch to use html-void-elements package?

[fisker]

And it's funny, three of them removed in HTML spec... https://github.com/wooorm/html-void-elements/pull/7/files

[NullVoxPopuli]

oh I like that

@chancancode I can do this switch in a separate PR so the PRs stay focused

[fisker]

html-tags is even popular.. But I suggest use html-void-elements since the owner is more active in related projects.