digitalbazaar / rdf-canonize

An implementation of the RDF Dataset Normalization Algorithm in JavaScript.
Other
22 stars 13 forks source link

Fix escaping. #58

Closed davidlehn closed 1 year ago

davidlehn commented 1 year ago

NOTE: Escaping causes a serious performance regression with the initial version here. An optimization and benchmarks are forthcoming in another PR. This checkpoint is here for comparison and testing.

Tests available: https://github.com/w3c/rdf-canon/pull/81

dlongley commented 1 year ago

If there is a significant performance hit as you say, I think we will want to benchmark an option that does a quick scan for the need to escape control chars, etc. -- and only if it is needed does it run. Having such an option include that kind of information in the output as some meta data would also be helpful in the canonize algorithm because it could cause a switch in the comparator function as well (fast native string compare vs. explicit custom unicode code point compare).

codecov-commenter commented 1 year ago

Codecov Report

:exclamation: No coverage uploaded for pull request base (main-with-3.4.0@788e0f5). Click here to learn what that means. The diff coverage is n/a.

:exclamation: Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@                Coverage Diff                 @@
##             main-with-3.4.0      #58   +/-   ##
==================================================
  Coverage                   ?   90.09%           
==================================================
  Files                      ?       10           
  Lines                      ?      626           
  Branches                   ?        0           
==================================================
  Hits                       ?      564           
  Misses                     ?       62           
  Partials                   ?        0           
davidlehn commented 1 year ago

Merging to main staging branch along with the testing updates to have a testable base for other work.