Some thoughts on the future Native binding

[am11]

In the forthcoming release of libsass (v3.3.0), the structure of .vcxproj file that resides in the upstream has changed: https://github.com/sass/libsass/pull/1439. It now produces a .dll and .lib (it used to produce sassc.exe, which is now refactored to sassc repo where .exe belongs: https://github.com/sass/sassc/pull/132).

What this mean for libsass-net is there are (at least) two approaches that can be taken for enhancement:

1. Restructure the libsass-net/libsass/LibSass.vcxproj by removing all the native/* compile targets in favour of <import> native/win/libsass.targets.
   • I have not figure out yet how to specify CompileAsManaged=false for entire group of sources, imported by .targets in git submodule scenario (i.e. without modifying .targets itself).
   • Benefit: separation of concern - libsass-net will not have to update binding layer for each release of libsass unless there are interface/API breaking changes, which seldom happen.
2. Use shim approach taken by dotnet/corefx team and ditch C++/CLI binding all together and use DllImport[] directive in C#, to import the upstream functions and properties to be consumed. This means the native/win/libsass.vcxproj will be build as part of the build step (<Exec>'ing out the MSBuild task directly in .csproj file) or added to libsass-net solution directly and then DllImport'd in C# code without intermediate binding LibSass.vcxproj.
   • Benefits: in addition to the aforementioned benefits, libsass-net will be able to target Unix platforms via Mono (libsass.so is already distributed and build on many Unices). This may also provide significant performance boost (https://github.com/darrenkopp/libsass-net/issues/25).

Note (for option 2): (as mentioned in the Goals section) with shim approach the inherent benefit is; the ABI varies for each platform (libc for example), but the API will remain consistent for most part. This is aligned with what @mgreter and @QuLogic envisioned for libsass' new C API: https://github.com/sass/libsass/issues/631.

[RichiCoder1]

I'm not overly familiar with this project, but could you combine Approach 2 with ClangSharp's PInvoke Generator?

[aviatrix]

@RichiCoder1 interesting idea, @am11 @darrenkopp as i'm not familiar with p/invoking stuff, would this be a viable option for 3.3 ?

[am11]

Thanks @RichiCoder1!

I am not familiar with that project either, but it seems like it makes the binding hooks Clang-friendly, which is great and would be an extra plus if it also helps GCC and MSVCR scenarios. =)

@aviatrix, @darrenkopp,

Firstly, this is a technical decision; if moving forward, libsass-net is required to be "CoreCLR friendly" (which is something trending these days), then we would need to make sure that ClangSharp is also new .NET core compatible. If we take the route which .NET core team has taken (approach 2 + fixing ApiPortabilityAnalysis isues), then it would most probably make libsass-net CoreCLR-friendly, targeting multiple platforms and architectures.

Secondly, (IMO) less dependencies / layers are better, unless of course there are insurmountable benefits.

Having said that, once 3.3 gets released, lets try out multiple approaches to enhance the native binding layer. If we are able to auto-marshal the strings without the extra vcxproj layer (using DllImport etc.), later on we can enhance on top of this bare-bone implementation, incorporating Clang hooks etc. if necessary.

Lastly, there is one more dimension of enhancement; letting users define custom importers and custom functions in C# and then use it in Sass file. This will register a C# function with native (libsass) Sass_Function factory. But that part would drastically thickens the binding layer, and it would be well-maintainable, if this binding layer is written in pure C# (with probably bit of unsafe, DllImport etc. code blocks). For instance, in node-sass we used to have one cpp file as a binding layer and as soon as the advanced API features (custom functions/importers etc.) were implemented, the amount of binding code increased -- introducing the complexity.

[RichiCoder1]

I am not familiar with that project either, but it seems like it makes the binding hooks Clang-friendly, which is great and would be an extra plus if it also helps GCC and MSVCR scenarios. =)

Rather should clarify that ClangSharp PInvoke Generator simply auto generates C# .cs PInvoke wrappers using the target library's headers. That's where the dependency on clang ends. That being said, it does introduce a dependency on clang to be present in the first place when you want to generate the wrapper. This could be a manual, per libsass, release thing, or an automated CI job.

Beyond all that, well said @am11. Agree with all of what you said.

[am11]

@RichiCoder1, oh nice! I thought it is a permanent dependency, but it is a one-time generator tool. Awesome! :+1:

[RichiCoder1]

Yup yup. Will make creating the wrapper in the first place much simpler. Then you can create a wrapper around the wrapper (:D?).

[darrenkopp]

The plan going forward is definitely to use P/Invoke rather than the managed c++ stuff

[am11]

Update: I have converted the project with P/Invoke in my master branch https://github.com/am11/libsass-net/tree/master (will refactor the commits once all the features are added). At present, it is work in progress as I am in the middle of covering the libsass auxiliary features such as' custom importers, custom plugins, custom headers and custom functions.

Following is the list of planned tasks with current status and scribbled comments:

• [x] Implement core features with P/Invoke:
  • [x] UTF16 to (ANSI cp) UTF8 conversion and vice versa. :zap:
  • [x] Implement Sass options.
  • [x] Sass Compiler.
  • [x] Sass Result.
• [x] Implement custom importers; implemented with a delegate, works like .. as it should the C# way! :)
  • working example at https://github.com/am11/libsass-net/blob/288031a/contrib/LibSass.NET.CommandLine/Program.cs. Will update the usage docs later.
• [x] Implement custom headers (almost the replica of custom importer, the only difference is the execution-time, i.e. libsass invoke this once at the start of process).
  • This is an experimental feature in libsass, I will annotate it as experimental too ([Experimental] attribute?) in libsass.net for the wrapper API consumers.
• [x] Implement custom functions (this requires implementing Sass Types, figuring it out indulgently the best way to go about it).
• [ ] Implement custom plugins (need to research more on feasibility with MEF droppable plugin support).
• [x] Configure build process for x86/x64 to deliver managed code as true _{(pseudo? as we don't have ARM support :smiley_cat:)} AnyCPU configured.
  • This is done with msbuild tasks, which compile the libsass solution (located at libsass-net\src\libsass\win\libsass.sln) twice with both x64/x86 configurations when libsass.net.sln is built.
  • This way it caches the build history, so devs don't have to experience delays every time they build libsass.net.sln.
  • This also means libsass is totally isolated from libsass-net, as libsass-net is now a true 'foreign function interface' wrapper implementing libsass API's exported functions (as opposed to relying on internal functions of libsass, and statically compiling libsass as part of build).
  • In libsas.net sln, there is no reference to libsass.vcxproj. We can still step through it as all the meta data and mapping (pdb) files etc. are available for debugging. Just press F11 and you can step through P/Invoked function in cpp. :star:
• [ ] Configure Nuget packaging using runtime format as described here: https://docs.nuget.org/consume/projectjson-format#runtimes. Later when .NET core support is implemented, we can use the same approach to pack xpalt libs (libsass.so, libsass.dylib etc. AFAIK, this is the same approach what dotnet folks have took for packaging corefx/src/Native).
• [ ] Unit tests; will cover all the core and aux features separately.
• [ ] Integration tests; will cover core + aux features combined.
• [x] Sass-Spec tests (need to create a loop runner with file context like node-sass).
• [x] Contributors projects:
  • [x] Update Web project.
  • [x] Implement a Console application.
• [ ] Add+update docs.

Currently, the libsass submodule is pointing to tip of am11/libsass master, which is one commit ahead of sass/libass with an additional fix: https://github.com/sass/libsass/pull/1821. That will most probably get fixed with a better approach as per @mgreter's comment. At which point, I will rewire it back to sass/libsass's future release tag.

[aviatrix]

awesome work @am11 , keep us posted and let us know if we can help with something :)

[am11]

Thanks @aviatrix! I will surely keep you guys posted and keep updating the task list. :)

[am11]

Custom Headers (aka mixins) are implemented via https://github.com/am11/libsass-net/commit/d5b836c. From implementation and usage standpoint, it is exactly the same as custom importers, but semantics and implications are different. For details, please take a snap at https://github.com/sass/libsass/issues/960 and https://github.com/sass/libsass/pull/1000.

As I said above, since this is an experimental feature in LibSass, hence LibSass.NET should probably inform the consumers via [Experimental] attribute which will act like [Obsolete("This feature is not well tested")]. Based on https://github.com/dotnet/roslyn/issues/156#issuecomment-72249480, it is possible to implement it with analyzer, but that can be implemented later if necessary. Meanwhile, the documentation comment says (Experimental).

---

Next feature is Custom Functions, which require implementation of several SassTypes in C#. I have planned to add two separate interconvertable interfaces: public ISassType (for consumers) and private ISassExportableType (for Libsass). Libsass.net will call ISassExportableType.GetInternalTypePtr on each returned object in libsass fn callback, which would call libsass back in return to instantiate the appropriate type in unmanaged space.

The reason ISassExportableType with ISassExportableType.GetInternalTypePtr() will not be public is to avoid malicious invalid pointers injections. Thereby, I will leave the types inheritable (unsealed), but it would be an error if the object returned by consumer is not convertible to ISassExportableType, i.e. if someone implements ISassType, that will not work, it has to be either the same prototyped object or extension to the provided types. (since there is no such thing as sealed interface or sealed interface inheritance in c#: https://github.com/dotnet/roslyn/issues/6869 :smile_cat:)

Note that if the returned object is not convertible to ISassExportableType at callback-time, It would result as an error -- which will look and feel like regular LibSass compiler error to end consumer -- not a runtime exception. So basically we will have exception handling done for user and in catch block create a sass error object with message saying "The return type must not be an arbitrary implementation of ISassType. Please use either be predefined Sass types or inherit one for extended functionality."

Feel free to share your thoughts. I am personally not a fan of catching too many exceptions in API to defend users mistake, but this case seems necessary to avoid breakage. I will start this implementation over the weekend.

[aviatrix]

Catching Too many exceptions in the api isn't a good practice in my view either, however we should definitely give a good exception message when the person writing scss poops things up :)

[am11]

Added initial support for Sass types and SassTypeException. https://github.com/am11/libsass-net/commit/5afa5d634c6015b22aeeac7adf8753a9ed0913b7

In LibSass, the type Sass_List accepts the value of type Sass_Value, which is essentially any Sass type. In C#, this means all Sass types first implement ISassType interface. Then SassList has a property Values of type List<ISassType>, which is vulnerable to circular references in case user adds List<SassList> to values list of another SassList which may point back to same instance. To remedy that, I have added WalkAndEnsureDependency method which recursively walks the nested list and detects this specific loop pattern -- which is fatal due to possible infinite allocations on LibSass heap via ISassExportableType.GetInternalTypePtr() -- as anomaly and throws SassTypeException with a pretty message.

Unfortunately, this solution does not fix the problem entirely as there is another list-y SassType called SassMap, which is prone to the same circular-reference problem. :warning: More fun to the mix would be cases such as a SassList ListA containing a SassMap MapA which contains another list ListB with a member ListA; this kind of corner cases needs to be tackled and tested before going to production IMO.

I will work on SassMap (and few remaining types such as Sass_Bool) on next weekend. In a zoomed-out bigger picture, SassTypes stuff is part of custom functions feature (and would also be used in Custom Plugins, which IMO needs more investigation for MEF compatibility and its availability in CoreFx and can be incorporated separately).

[darrenkopp]

great work, currently reviewing your commits, will add comments there.

[am11]

Thanks @darrenkopp! I will incorporate those changes soon.

[am11]

Incorporated Darren's feedback (modulo the copyright headers in some files; which will be handled in the final stage before the PR): am11@dd9a1eb

Random notes:

• SassMap type: http://sass-lang.com/documentation/file.SASS_REFERENCE.html#maps is a key value pair with both key and value of type Sass_Value, where Sass_Value being any sass type aka ISassType on C# side. Hence SassMap in C# is implemented with Dictionary<ISassType, ISassType>. Although it is not quite self-explanatory from the ensure method implementation, but I think the code path that leads to this recursive ensure method covers all the nodes for the given map, as the caller of WalkAndEnsureDependencies, the GetInternalTypePtr is also recursive. :smile: (in other words, I haven't tested the code around ensure ... yet :wink:)
• Since the type pointers are stored internally, we need to have some mechanism to invalidate them, in case consumer tries to reuse the same instance across multiple compile()s (among many other cases). Note that de-allocation of native side memory is not handled in our wrapper, since libsass will free() all the allocated resources (verbatim term from libsass code) pertaining to type etc.
  • We need to have an event notification in SassCompiler, which every ISassType implementer type will subscribe to; so where SafeSassContextHandle is disposed and calls libsass to delete the context, it will fire the event (say SassTypeInvalidatedEventNotifier) to notify all the types to nullify their internal ptrs and reset ensure (applicable to list-y types only). The next use of type will re-allocate the native memory on libsass heap. The goal is to prevent consumer apps from breaking in every possibility (or simply throwing canonical error if it is too much work).
  • All types' Value property setter will also fire SassTypeInvalidatedEventNotifier.
  • SassList and SassMap would probably end up implementing IList<ISassType> and IDictionary<ISassType, ISassType> respectively. This is because whenever user will make any change in the Sass[List/Map].Value list/dictionary (via Add, Insert, Remove, index operator etc.), depending on the method, it would need to either invalidate the list and resync or update the list with the native side.
• Two types SassNull and SassBool will be created as internal providing singleton instances. We will reuse those instance for custom functions and plugins as well as importers, headers (the latter two are creating null instance via p/invoke for every return null from consumer at the moment).
• SassColor type is still TODO.

[aviatrix]

@am11 Memroy : current biggest issue we have with current implementation of libsass is that when IIS recycles the app pool something doesn't go as planned and every subsequent request which includes an scss compilation takes forever(1-2 minutes) to complete and 100% of your cpu; do you think this will be mitigated with those SassTypeInvalidatedEventNotifier ?

[am11]

@aviatrix, the notification event would be used for Sass type-system, which is meant only for Custom Functions/Importers/Headers/Plugins, if used by user in the options object.

However, regarding the memory issue, there are couple of things:

• One of the recent versions of LibSass introduced performance degradation which was mitigated in the latest more recent version, but comparatively the latest version performs bit slower than v3.1, which is probably by design; given new language features are continuously being incorporated.
• The overall rewrite with P/Invoke will at least provide us more straightforward control over the memory in terms of which side is owning the memory and who is responsible to free it.
• We are using SafeHandle derived classes (to provide a secure and better way than IDisposable to deal with the raw pointers, aka IntPtr/UIntPtr), which get disposed once the call leaves the scope (like any .NET type including IntPtr) with the exception that SafeHandle.ReleaseHandle() overridden method is called, where we P/Invoke libsass delete context (code). This way we have kind of an interface promise about the life-cycle of context and all the encapsulated pointers. However, we cannot reuse libsass context after one compile operation and (like current libsass.net, node-sass and perl-sass), we should delete the context and create a new one for the next call to SassCompiler.Compile().
• Although I believe that there will be a performance gain in all dimensions of libsass.net with p/invoke implementation, we can further it by make the ISassOptions objects reusable for consumer with reliability. The reason I am calling this out is that in current libsass.net, every time the compiler.Compie() is called, the options sub props reallocate memory on the CLI side of wrapper and then copied further to libsass, so regardless the C# consumer reuses the options across the compile calls or create a new options object for each, it will always allocate the memory on all tiers. With P/Invoke, although we are also making a lots of copies of strings, we tend to quickly leave the scopes and let them auto-dispose (though I haven't tested the behavior with Server GC yet) and then pass directly to libsass. LibSass then also does a lots of memory movements and allocations internally during the compilation (which, going by their plan about improving memory management, will get improved in future).

One thing that @mgreter mentioned on Slack was that libsass might support UTF16 mode (as an option or automatic for Windows), which will save us from making too many conversions.

[aviatrix]

@am11 thank you for the clarifications ! It's much appreciated !

[aviatrix]

Hey @am11 , any progress with this? Is there any way we can help ? Maybe write tasks with what must be done ?

Best, Nikola

[am11]

Hey Nikola, sorry for the super delay. I was very busy with life/job stuff, so I could not continue on libsass.net. The Sass type system is still work in progress. I will spend time on this weekend to revisit the progress on this and update here. Recently, @mgreter implemented one required piece for foreign function interface (https://github.com/sass/libsass/pull/1821), via https://github.com/sass/libsass/pull/1983. This means that we are zero blockers from libsass side at the moment.

[aviatrix]

@am11 awesome news ! Looking forward to this !

[am11]

@aviatrix, once again truly sorry; the weekend did not went as planned. I started another port and was busy finishing that one. I will resume the SassList and SassMap type-system and dependency walker this week and keep this port going. :)

[am11]

I have made some progress in type system. All the required bits are in place to enable Custom Function feature.

The custom validity notifier event is implemented, which when fired, resets the internal pointer of all the sass type objects.

However, the mutability aspect of types, in terms of reusability of sass objects, is quite unsettling at the moment and perhaps only need thorough tests and detailed documentation enlisting the corner cases. For instance:

SassNumber _sassNumber;

SassList MyCustomFunctionA(double seedValue)
{
  _sassNumber = new SassNumber{ Value = seedValue }; // TODO: add ctor-> new SassNumber(10.01)
  var list = new SassList();

  list.Values.Add(_sassNumber); 

  _sassNumber.Value = 44.7;

  list.Values.Add(_sassNumber);

  return list;
}

SassNumber MyCustomFunctionB()
{
  _sassNumber.Value = 12.321;
  return 
}

void Process()
{
  List<CustomFunctionDescriptor> functorList = new List<CustomFunctionDescriptor>();

  functorList.Add(new CustomFunctionDescriptor{
    SignaturePrototype = "foo(seedval)",
    FactoryDelegate = () => { return MyCustomFunctionA(10.01); }
  });

  functorList.Add(new CustomFunctionDescriptor{
    SignaturePrototype = "bar()",
    FactoryDelegate = () => { return MyCustomFunctionB(); }
  });

  var myOptions = new SassOption();
  myOptions.CustomFunctions = functorList.ToArray();
  // set other options

  Console.WriteLine
  (new SassCompiler(myOptions)
     .Compile()
     .ToString());
}

This will eventually end up having 12.321 on all three _sassNumber accesses. This is a simple example where ISassType object is shared. It will get complicated if the user alter the internal property (Values in this case) using reflection.

Now if we continue reusing the sassOptions object:

  // set other options

  Console.WriteLine
  (new SassCompiler(myOptions)
     .Compile()
     .ToString());

 // add another function
  functorList.Add(new CustomFunctionDescriptor{
    SignaturePrototype = "echo(x)",
    FactoryDelegate = () => { return MyCustomFunctionA(33.44); }
  });

  myOptions.CustomFunctions = functorList.ToArray();

  Console.WriteLine
  (new SassCompiler(myOptions)
     .Compile()
     .ToString());

The second compile will result in 44.7 in all places.

Hence, the reused SassType objects are mutable per compile session and immutable across the compile()s.

(these are the current state of affairs, but I am open for suggestions)

---

Next I will continue to actually implement CustomFunctions, as CustomFunctionDescriptor in the aforementioned example is non-existent atm. 😄

[aviatrix]

Got you. How is the functionality implemented across different ports like node.js & ruby ? What else is on your to-do list to reach "good enough to use in production" ?

[am11]

JavaScript custom objects are fundamentally mutable, Ruby has a dynamic type system, so I think we need to abide by general laws of C# flavored strict OOP.

I am in a process of implementing SassFunction. So far there is a SassFunctionCollection derived from List<SassFunction> with string indexer, which means from the end-consumer point of view, using C#6 collection initializer, they can do something rhetorical like:

sassOptions.Functions = new SassFunctionCollection 
{ 
    ["foo(blah, param2)"] = (options, argv) => { /* do something with argv */ }
}; 

At the moment, argv is of type params ISassType[] and anonymous function is returning single ISassType, which makes it pretty tough to introspect the actual derived type returned at "registration time". Some dark arts of reflection / Roslyn-CompilerService required, as the anonymous delegates return ISassType and we need to make the exact Sass_Value derived type upstream corresponds to what consumer has returned.

For this binding part, where LibSasss.NET will register the user-defined SassFunction delegates LibSass is left, which requires deliberate effort. I think when registering the types with LibSass API, we would also be able to invalidate pointers at that point to mitigate the mutability consistency issue. But there will still be some grey areas but much better than the current situation.

What else is on your to-do list to reach "good enough to use in production" ?

The SassFunction is a center piece and is the most advanced feature LibSass has to offer. I wanted to complete this feature before moving to unit tests (without intensive unit testing, it is a no-go IMO). After unit testing, it will be good to go (meaning SassPlugins can be shipped in a later release).

[am11]

SassFunction are incorporated. There will be some bugs somewhere, but it is working out quite well with the console app situated at contrib/LibSass.NET.CommandLine. Therefore, I have marked it done in the checklist and will fix the bugs as they emerge. 😺

Next is unit testing and spec testing. I will start with some rudimentary handwritten fixtures before making the runner for SassSpec submodule (https://github.com/sass/sass-spec).

[am11]

I have added SassSpec runner. All 975 specs are passing: as of https://github.com/sass/sass-spec/commit/a9a26f4ba268cfaa0808a9ebde90b03944aef895 <- corresponds to libsass v3.3.6. I will add couple of unit tests, squash commits and prepare the PR next week.

[aviatrix]

Hey @am11, Great work! Thank you so much !

Last night i spend some time playing around with the library from your repo. I had to do some tweaking to the git repos as for libsass it was pointing to a fork of yours and wouldn't checkout it as it was pointing to a commit which wasn't there... anyway... i finally got it to work, and it compiles both bootstrap4 and foundation zurb.

One thing that i noticed that could be fixed was the namespaces they were Sass.* and not LibSass.* , Any particular reason for this or this is going to be renamed ? i could do that too after the PR if you want

[am11]

Hi @aviatrix, I fixed the submodule path last night (my last commit was about fixing that and setting to 3.3.6 version hash). I will fix the namespace. Thanks for calling it out.

[am11]

CI is now showing the test runs too: https://ci.appveyor.com/project/am11/libsass-net/build/1.0.139/job/659676hdwhhav54p/tests. 🎋

@aviatrix, could you please explain how were you getting the LibSass.* namespace? We have Sass as the default namespace (LibSass.NET.csproj#L10) and all the classes follow this namespace.

[aviatrix]

@am11 hmm... i expected the namespace to be LibSass since the library is called the same... haven't touched the library in a while, so i must have forgotten this.. my bad.

[am11]

Ops, I parsed it the other way around. I though you were asking about renaming it from LibSass to Sass. wilco! 😄