adrianhelvikspond commented on PR #3014:
URL: https://github.com/apache/thrift/pull/3014#issuecomment-2266152453

   **1. How does that runtime lookup affect performance?**
   
   I did a quick benchmark here, and using a pre-constructed symbol was equally 
fast as using strings, while constructing the symbol on the fly like I did take 
about 10s/billion calls.
   
   <details>
   <summary>Benchmark code</summary>
   const ONE_BILLION = 1_000_000_000;
   
   const uniqueSymbol = Symbol.for("read");
   const symbol = Symbol("read")
   
   const symbolEntity = {
        [Symbol.for("read")]() {
                return "Hello world";
        },
   };
   
   const constSymbolEntity = {
        [symbol]() {
                return "Hello world";
        },
   };
   
   const constUniqueSymbolEntity = {
        [uniqueSymbol]() {
                return "Hello world";
        },
   };
   
   const stringEntity = {
        read: () => {
                return "Hello world";
        },
   };
   
   for (let i = 0; i < 100; i++) {
        time("symbol constructed every time", () => {
                for (let i = 0; i < ONE_BILLION; i++) {
                        symbolEntity[Symbol.for("read")]();
                }
        });
        time("const symbol", () => {
                for (let i = 0; i < ONE_BILLION; i++) {
                        constSymbolEntity[symbol]();
                }
        });
        time("const unique symbol", () => {
                for (let i = 0; i < ONE_BILLION; i++) {
                        constUniqueSymbolEntity[uniqueSymbol]();
                }
        });
        time("string", () => {
                for (let i = 0; i < ONE_BILLION; i++) {
                        stringEntity["read"]();
                }
        });
   }
   
   function time(name, fn) {
        const start = process.hrtime.bigint();
        fn();
        const end = process.hrtime.bigint();
        console.log(name, format(end - start));
   }
   
   function format(time) {
        // Nanos to micros
        time /= 1000n;
   
        if (time < 1000n) {
                return `${time}µs`;
        }
   
        // Micros to millis
        time /= 1000n;
   
        return `${time}ms`;
   }
   </details>
   
   ```
   symbol constructed every time 9876ms
   const symbol 471ms
   const unique symbol 486ms
   string 478ms
   ```
   
   So while I doubt it's going to be the performance bottleneck of most 
applications, it's easy enough to store the symbol in a variable before using 
it. So I'll fix that.
   
   Regarding prefixing, the way JavaScript introduces standardised symbols is 
that they use `Symbol("...")` (globally unique) instead of `Symbol.for("...")` 
(unique to the passed in string).
   
   `Symbol.iterator` or `Symbol.asyncIterator` are examples of that. You 
categorically cannot construct these symbols yourself. So while somebody may 
create a symbol that is equal to `Symbol.for("read")` it will never be a 
problem. I'd liken it to somebody subclassing a Java thrift class and 
overriding the `.read` method vs not being able to do that. Possibly useful to 
be able to override the method for testing purposes, but not in any other case.
   
   Security-wise I'd argue that symbols are a teeny tiny bit safer than strings 
as you cannot deserialise a value into a symbol with any of the thrifty 
serialisers or even JSON. But there's no real difference. Symbols are 
essentially just a different class of strings that can be globally unique if 
you use `Symbol("...")` instead of `Symbol.for("...")`.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to