POC: Tag the code blocks based on features

[pm-harshad-mane]

Type of change

• Feature: Open to discussion, DO NOT MERGE.

Problem

• Prebid.js has become bulky
• Not all publishers need all the available features
• The final build has the code with all supported features that pub may not want
• This increases the build code size
• All bidder codes are written in a way that the bidder will support as many features as it can
• Example: If publisher does not want to use PBJS for Native ad rendering then PBJS should be built w/o Native related code
• Refactoring bidder codes to support such needs is difficult
• Need a simpler way to reduce the code size

Solution

Using https://www.npmjs.com/package/gulp-remove-code plugin, please go thru its small documentation
Introducing removeCodeConfig.json

{
	"disableNativeRelatedCode": false
} 

when above flag is set to true then code like below will be removed in build process before code is minified

// removeIf(disableNativeRelatedCode)
import { processNativeAdUnitParams, nativeAdapters } from './native.js';
// endRemoveIf(disableNativeRelatedCode)

Please note that we will also need to tag the respective code references in the file, else code will crash at run-time.
It is bit risky to maintain our code this way but it is easier to do per file/feature basis.

In this PR i have made change for pubmaticBidAdapter.js and core code to support disableNativeRelatedCode and disableOutStreamRelatedCode flags.
disableNativeRelatedCode when set, it will remove native related code from pubmaticBidAdapter and prebid core.
disableOutStreamRelatedCode flag when set, it will remove the OutStream creative rendering related code in pubmatic adapter.

Impact

With flags set, we see 6KB of file size saving in minified final build.

gulp build --modules=pubmaticBidAdapter

## With flags set to “false”
harshad.mane@RWC-WS57 Prebid.js % ls -lrth build/dist/prebid.js 
-rw-r--r--  1 harshad.mane  staff   156K Dec  2 14:53 build/dist/prebid.js

## With flags set to “true”
harshad.mane@RWC-WS57 Prebid.js % ls -lrth build/dist/prebid.js 
-rw-r--r--  1 harshad.mane  staff   150K Dec  2 14:55 build/dist/prebid.js

Problems with the approach

• Need to check if we can handle removal of code scenario in unit testing as well

Conclusion

It may not be the best solution.
But it is easier to implement considering the code size.

[pm-harshad-mane]

Hello @jsnellbaker ,
Would like to hear from you about this feature suggestion.

[pm-harshad-mane]

Hello @mike-chowla ,
Could you please check this proposal?

[pm-harshad-mane]

Hello @jsnellbaker ,
Can you please review it?

[pm-harshad-mane]

Hello @jsnellbaker , @robertrmartinez , @snapwich , @Fawke
Can you please review this PR, this feature if included in build process it can reduce the payload of prebid.js in significant amount. PubMatic wants this feature to be part of the build process, it is optional in build process so it will not have any impact if not enabled.

[pm-harshad-mane]

Hello @jsnellbaker , @robertrmartinez , @snapwich , @Fawke ,
We are waiting for your review/feedback for last 12 days!
CC: @bretg , @mike-chowla

[jsnellbaker]

@pm-harshad-mane To be honest, I don't know how I feel about this. While the approach could work, I'm not sure how extensible it would be when it's spread across the whole project, especially depending on how many variants/configs we'd plan to support.

What combinations/how many were planned?

[bretg]

Interesting idea, but I think this one needs some discussion. @gglas, please bring up with PBJS group and @patmmccann with the publisher team?

The proposal looks to me like being able to slim the size of PBJS packages by mediatype.

Unclear to me how many bytes pubs could be practically expected to save or how many bid adapters we're going to get to make these changes.

[pm-harshad-mane]

Hello @jsnellbaker , @bretg , @patmmccann , @gglas
I was on leave for some time so replying late.
Initially we can target mediaType based builds (4-5 flags), we can extend if needed.
In ideal world our code should have been structured based on media-types but it is not and its an enormous effort to restructure.
Apart from media-type based code, other features are built in modules so we do not need tagging for those.

For current PR i have mentioned the Impact in description, posting it here again
With flags set, we see 6KB of file size saving in minified final build when only pubmatic bidder is selected in the build.

gulp build --modules=pubmaticBidAdapter

## With flags set to “false”
harshad.mane@RWC-WS57 Prebid.js % ls -lrth build/dist/prebid.js 
-rw-r--r--  1 harshad.mane  staff   156K Dec  2 14:53 build/dist/prebid.js

## With flags set to “true”
harshad.mane@RWC-WS57 Prebid.js % ls -lrth build/dist/prebid.js 
-rw-r--r--  1 harshad.mane  staff   150K Dec  2 14:55 build/dist/prebid.js

I can add tags in most used bidders in incremental commits.

Some more stats, I hosted PBJS basic example with only AppNexus bidder (v4.20) here https://pm-harshad-mane.github.io/PBJS/basic.html and Chrome Dev tools shows that pbjs has 62.5% unused code in pbjs.js 97793 bytes un-used out of 156537 bytes.

We at PubMatic strongly believe that we need to improve PBJS on download size as it impacts publisher page performance.

[dgirardi]

I am coming late to this discussion and I'm not sure if this is all of it, but there's a couple things I'd like to note (@jsnellbaker, @patmmccann):

• we are already removing dead code as part of the minify step (tersify does it by default: https://www.npmjs.com/package/terser - dead_code (default: true) -- remove unreachable code. I mention it because I saw some notes hoping that upgrading webpack would help, I think that's unlikely.

• The reason this approach does reduce bundle size is that it removes live code that happens to never get run. I agree that it seems hard to maintain. To me the main issue seems to be that a lot of code looks for particular settings or request/response values that keep code alive, even if you (as the publisher) know they will never be there. I think a better approach would be to programmatically put "breakpoints" around access to those values. For example, instead of:

       switch (bid.mediaType) {
             case NATIVE: doNativeStuff();
       }
  

  we could have something like:

    bid.ifNative(doNativeStuff);
  

  and define ifNative based on build settings. This would still be a major refactor, but it has the advantage that it would automatically "cascade" dead code detection to all dependencies, you wouldn't have to (for example) care about imports that only get used inside doNativeStuff like you do here.

[ChrisHuie]

closing this pr as I believe it is now outdated. There are a few ways to approach this though and know it is on the more immediate roadmap.

[bretg]

Re-opening until there's a link to another issue.

[bretg]

Opening an issue to track